Jump to content

er314

Members
  • Posts

    44
  • Joined

  • Last visited

Posts posted by er314

  1. GetFieldValue is responsible for performing access control to a field value, and denying access when applicable.

    I would like to be able to log the access denied events (on Dev, for debugging purpose ; in Prod, for security monitoring purpose).

    -> basically, I would need to do, via hook, the equivalent of the following addition :
     

    Page.php
    [...]
    protected function getFieldValue($key, $selector = '') {
    [...]
            if($field->useRoles && $this->outputFormatting) {
                // API access may be limited when output formatting is ON
                if($field->flags & Field::flagAccessAPI) {
                    // API access always allowed because of flag
                } else if($this->viewable($field)) {
                    // User has view permission for this field
                } else {
                    // API access is denied when output formatting is ON
                    // so just return a blank value as defined by the Fieldtype
                    // note: we do not store this blank value in the Page, so that
                    // the real value can potentially be loaded later without output formatting
    /*** CODE ADDED ***/
    if ($this->wire()->config->is_logging_access_denied) {
        $log = $this->wire()->log;
        $log->save("errors", "Core access denied - field {$field->name}, template {$template->name}, page {$this->id}");  // + user & URL are auto-added to the log event
    }
    /*** ***/
                    $value = $fieldtype->getBlankValue($this, $field);
                    return $this->formatFieldValue($field, $value);
                }
    [...]

     

    Question : Is it possible to somehow hook into this getFieldValue() method ? (in order to perform the above added processing via hook)

    I don't think so (according to https://somatonic.github.io/Captain-Hook/index-dev.html), but I'm asking, for possible ideas.

     

  2. Hi, 

    I'm resurrecting this old thread, because it's spot-on for my issue :

    Rather up to date PW version here (3.0.153.

    With the above code, my repeater field eventually contains 3 new items (all identical), instead of 1 !

    More precisely :

     

    // here, $page->buildings->count() returns 0  (all right)
    $building = $page->buildings->getNew();
    $building->title = 'One Atlantic Center';
    $building->blabla = 'test';
    $building->save();
    // here, $page->buildings->count() returns 1  (that's understandable)
    $page->buildings->add($building);
    // here, $page->buildings->count() returns 2 (ouch !)
    $page->save();
    // here, $page->buildings->count() returns 3 (ouch !!)
    // and in the admin GUI, there's 3 new items (all identical)

     

    -> Has anyone noticed this behaviour ?

    As a workaround, I may skip the ->add() step (despite that this step is the API-documented way of doing things),
    but even then, the $page->save() is still creating a second identical item ; and this is a problem...

     

     

     

     

  3. Hi,
    sorry if the topic is covered elsewhere.
    Background : I'm a part-time, for hobby (ie. not-for-a-living) processwire developer,
    so I'm probably not very representative of the community,
     

    Question : I'm wondering what is the "governance" status of processwire, these days ?
    With this question, I mean that let's say we segment processwire as follows :
    1- pw core / roadmap / Ryan new features
    2- pw core / roadmap / community features requests
    3- pw core / issues fixes
    4- pw websites / main site
    5- pw websites / forum
    6- pw plugins ecosystem / Ryan commercial extensions
    7- pw plugins ecosystem / community free + community commercial
    8- pw weekly
    ...
    My understanding of the current governance status is :
    1- : This is Ryan's soverain decisions (for very good reasons ? , with addition of some core members work (on some pull-requests).
    3, 4- : This is also Ryan driven ; with several core community members doing the diagnosis & triage of issues
    All the above activities (1, 3, 4), along with activity 2-, basically, are funded by the community purchasing Ryan commercial extensions (6).
    5-, 8- : These are mostly run by several core community volunteer members.

    Now I'm wondering about 2- :
    What is the governance system for defining pw roadmap ? for "injecting" the community feature requests into pw roadmap ?
    Is there some kind of "board" of core members + Ryan, for deciding on the prioritization of these things ? or something like that ?
    Is there some kind of baseline defined for the breakdown of work on core activities 1- vs 2- vs 3- ?
    As a not-for-a-living user, I don't have any special nor legitimate expectations about pw roadmap decision-taking (besides having a better understanding of what becomes a feature request which one deposits on github ?
    But I guess the expectations must be different for other people, who are financially involved in pw ecosystem.

    And as a side note, I'm also wondering about 7- , as my feeling is that a frontend merging of 6- (which would remain prime) and 7- would, I think, be key for a broader pw outreach beyond the current adopters, for the benefit of the whole community ; something similar to eg. https://www.opencart.com/index.php?route=marketplace/extension

    thanks for your insights

     

    • Like 3
  4. er314 sorry I think I misunderstood, as I thought you were suggesting we change the behavior of the existing $pages->get() method

    Yes, this was precisely my original intent.

    But once you explained that it was not possible due to the big impacts on the Core (and there is no front-end vs. back-end state as you point out) , then your hook suggestion was a good solution for my usage, and I'm happy with this. And I know that any addition to the API have to be done very cautiously, so that it remains focused and consistent.

    Now, having this findOne() API method seems actually excellent, because, even in terms of naming, it makes perfect sense of what it does, and it nicely complements the API. And yes I think it will really add clarity, especially for newcomers.

    • Like 1
  5. hello, 

    Thanks for your lastest explanations. 

    To be very clear, I think our discussion has derived a little, from the original question about get() behaviour, to more general approaches about security.

    Personally, I don't mean to force you, or the whole PW community for that matter, to create and make use of a new API method ; as I said before, regarding the original question, I reiterate that I'm very fine with having my own "getSec" method which I would build via a hook thanks to the code your suggested at reply #10.

    Thanks again. I'm marking this thread as solved.

    • Like 1
  6. No, I have absolutely no use case where this alternative proposal would be needed in place of the 1st method.

    I was just trying to see if it's simpler/cleaner than the 1st method.

    My rough guess was that, it might be cleaner from a public API perspective (no new method needed), but much more complex internally. You are just confirming this, in regards to mysql :-)

  7. mmm... again, we cross-posted.

    I guess my post will clear up most things, as when reading yours, I agree with everything *

    so I hope my explanations about my objectives & my strategy are clear ; I can't do better.

    * I disagree (heavily :-) on one point though : 

    In the more than 5 years of this open source project, I'm not aware of any instance where someone has misused a get() method and caused a security problem "

    Actually I don't understand why you say this, it mostly contradicts the rest of your purpose :-)

    Anyway, let's go : without speaking specifically about PW, I can see the following main reasons for not being aware of vulnerabilities is a given web app :

    1. the web app has no flaw (because very good design & development processes + developer awareness + reviews + not too big & too complex + etc... + point .2.)

    2. the underlying CMF does a good job at preventing and/or assisting the designer & developer against many kinds of flaws (mostly for the syntaxic ones, less for the semantic ones)

    3. the web app has not a huge traffic / not a huge potential "value" for compromise ; so apart from script kiddies -which would be stopped by .2.-, no talented attacker has the motive to try and break it

    4. the web app has not been subject to a thorough security audit

    5. compromise occured, but as not been detected hihi

    Our current discussion sits mostly in point .1. , where I'm trying to get the best out of .2. , in terms of how to adapt my processes. 

    The fact that PW core has a strong reputation in terms of security (strong sec. framework + no known vulns) is also a strong point for another reason : it keeps script-kiddies and general lurker away from the web sites built on this platform.

    That being said, to make the asumption that there's no flaw, because we are not aware of any exploited in the wild... !

    Well, this may be said for www.cnn.com , for your online banking app, at&t selfprovisioning, ... ok, they are constantly probed and shaked. You guess mainly from where in the planet, and it's confirmed by real world statistics.

    Look, if we take as example PW CommentForm.php as above.

    If you had forgotten the check $page->viewable() , there would be a security flaw. Detectable and exploitable, just by testing from the internet for data inputs, even without code review.

    I'm just taking this as a "pseudo"-real case.

    So, now :

    - Do you think, that all PW applications in the wild have all, and always, this kind of check in place where needed ? my experience (in webapps security in general, not PW related) tells me no. It's human. Some rare people are geniuses (I have the chance to tchat with one right now :-) , but most are human, with sometimes not the right background, sometimes not being given the right priorities, sometimes make mistakes, etc...

    And if you don't know a single case, I cant tell you that yes, you have one : me ! Until I did a full review, I had a few issues of this kind.

    - Do you think this would be detected in the wild ? clearly, to detect this, it means that it's not a script-kiddie, it means that it's a real person, with his brain, whose aim it to find a flaw in the logic of this particular application. Frankly, even for processwire itself, I'm not sure that, would this check be forgotten, someone would have discovered it as of today. So, for a given custom web app made with PW, well... it depend on the traffic. If more traffic than PW, then more chances. In can happen. or not. 

    So basically, I'm sure that yes, some web apps built with PW have security flaws. Semantic flaws, in their logic, in corner cases. Of course !

    And it's not PW fault. And there would be even more flaws if PW security foundation was not what it is.

    I have the impression that I'm saying obvious things, no ? :-)

    Actually I don't understand why we don't agree. And I don't even criticize PW in any way ; there I would have understood :-)

    • Like 1
  8. I don't get your point, except that I see that you don't get mine :-) , so I'll try to be as clear as I can :

    Frontend developer, is you and me, as opposed to PW core developer. I was making this distinction, as per Ryan explanation about get() having to remain as it is, unless would break PW core.

    So, what every frontend developer (i.e. web app developer), using PW,

    1. who wants to work in "security by default" mode,

    2. and who wants to take benefit of PW underlying security framework at its best,

    can do, is, as suggested by Ryan, 

    * for retrieving one single page

    -> use getSec() (which would best be a hook as proposed by Ryan above, because "security by default" should be easier to do, or at least not more complex, than "security bypassed") :

    $pages->addHook('getSec', function($event) {
      $page = $event->object->get($event->arguments(0)); 
      if($page->id && !$page->viewable()) $page = new NullPage();
      $event->return = $page; 
    }); 
    

    - unless you want to bypass the access control policy which is in place, and retrieve the page disregarding the access rights

    -> then use get() instead 

    Why bother with all of this ?

    -> again :

    - to take full advantage of the underlying security features of PW framework

    - to apply as many security measures as possible "by default" ; disabling a security control should be explicit, not implicit

    - to make use of the framework, as much as permitted, for avoiding coding errors / mistakes / unanticipated use cases & attacks scenarios

    This is the rationale.

    In a nutshell : for me, it's never use get(), unless explicitly justified ; use getSec() instead.

    This is what I call security by default, making full use of the security framework potential. This is how I aim to use any "software stack", and PW CMF in particular, and PW is rather well suited for this.

    Digression below ; feel free to tl;dr

    So, while I of course fully agree with "[it's about] how much access you as a developer are providing the potential user" , 

    I don't agree at all with "it's not about how secure pw's api is"

    -> all the contrary. One of the main "selling point" for me, for choosing PW, is exactly because of its strong security foundations. 

    In the current discussion thread, we are nitpicking about one particular PW API call, but PW security foundations are strong, and this is what counts, and this is what permits to very easily adapt the usage of the API to (my view of) what is the full potential of the PW security framework.

    Without a strong security foundation, it would mean that it's all up to the developer to 1. learn then 2. imagine and 3. handle all possible attack scenarios. This is simply *impossible*. In real life, this means "oh no we have been hacked ! it's the fault of this guy, the developer". It may be, but at some point, no, it's the responsability of the designer / decision maker.

    I think we can be glad that nowadays with current technologies web developers don't have anymore to handle buffer overflow issues ; nor cookie generation & handling issues, etc... And I am glad that PW provides the foundation and API for handling fine grained role-based access control (RBAC), for me it's a MUST, and PW implementation rocks.

    And now that, after some practice with PW, I have fully understood my need for the getSec() hook, I have nearly everything to feel confident and comfortable.

    Unless I'm mistaken of course. Or unless my English is too bad. Or unless both :-)

    Well at least this ends my arguments on this topic :-)

    cheers

  9. Or... another solution, for not adding the burden of needing new fields : have PW migrate to using unguessable IDs, instead of the current 1-to-N scheme.

    In practice, make all page IDs for newly created pages unguessable, à la youtube hashes.

    This would mean, for a fresh install, make all IDs this way. -> As an install option, for compatibility with legacy applications. And probably for some "root" IDs it would be hard and/or too much implications to follow this path.

    Yes, I know : even though at first sight things looks clean from the API side for going towards this path ($config->xxxxxPageID , $user->isXxxxx) , I know that things must be much more complex than this :

    1. there's the problem of legacy applications, which probably for most of them don't make full use of $config->xxxxxPageID  and $user->isXxxxx

        -> hence being optional at install time and at runtime

    2. there must be a huge number of implications in PW core ; and potentially in many modules.

    But hey, this is for opening the discussion. Probably another lost cause :-)

    cheers

  10. Ok, I get the point that allowing to change get() behaviour for frontend purposes would completely break PW :-) .

    And I understand that get() is not the method I need for my purpose. 

    My purpose is to give to frontend developers the ability to fully benefit from a strong underlying security feature of PW (let's call it "implicit viewable access control enforcement"), and currently by default it's not as straightforward as it could be.

    So while this strong underlying security feature of PW could offer by itself implicit prevention from a number of coding errors and security impacts, which would always be very welcome, currently by default it's not the case. So in a sense, the strong security framework provided by PW is not fully realized in terms on API.

    And I agree that my mind is not formatted the way it is expected to be (upside down), ever since the 1st time I read "if you are asking for a single specific page, we assume you mean it" , :-) which means check_access is forced to 0 on the most often used API call.

    And finally I agree that the API, and your previous suggestions (thanks for the hook method, it looks really nifty & optimized), provide all the tools needed to quite efficiently make things as straightforward and realized as can be :-)

    thanks

  11. Another example in the wild is PW's Comment submission workflow :-) :
    In CommentForm.php, after retrieving the just POSTed "page_id" input parameter, you perform a get() which uses this id, then you check that the resulting page is viewable before going further.
    So this is correct ; during the dev, you thought about "what if this parameter is tampered by a malicious user?", you arrived to the conclusion that access control must be enforced, so you have to check that the page is viewable, check added, perfect. 
    We are not worried about YOUR devs obviously :-) it's more about the devs made by the rest of the world :-)
    So here on this case, would this reasoning, and this additional check, had not been made by the developer -> bam, security vulnerability in the web app.
     
     
    To summarize, I think that what teppo & me are expecting, (well, at least me haha), in terms of developing practices & workflow/procedure, is :
    • for finding a set of pages
      • -> use find()
      • unless you want to bypass the access control policy which is in place, and retrieve all results disregarding the access rights
        • -> then use findNoSec() instead (which would be a hook, or a wrapper, or the developer having to add check_access=0 so that he's well aware that he's purposedly bypassing access control)
    • for retrieving one single page
      • -> use get()
      • unless you want to bypass the access control policy which is in place, and retrieve the page disregarding the access rights
        • -> then use getNoSec() instead (which would be a hook, or a wrapper, or the developer having to add check_access=0 so that he's well aware that he's purposedly bypassing access control)
     
    This would be possible if we had something like $config->get_is_like_find = true;  :-)
    But, it seems that we won't have it :-)
     
     
    So, based on the previous replies, the developing practices & worklow that we should put in place, will be more like :
     
    • for finding a set of pages
      • use find()
      • unless you want to bypass the access control policy which is in place, and retrieve all results disregarding the access rights
        • -> then use findNoSec() instead (which would be a hook, or a wrapper, or the developer having to add check_access=0 so that he's well aware that he purposedly bypasses security)
    • for retrieving one single page
      • -> use getSec() (which would best be a hook as proposed by Ryan above, because "security by default" should be easier to do, or at least not more complex, than "security bypassed")
      • unless you want to bypass the access control policy which is in place, and retrieve the page disregarding the access rights
        • -> then use get() instead
     
    So ok, this will do the job, but, I hope you better understand why it still does not fully map with our approach, it's still somewhat "upside down", and prone to unwanted mistakes.
  12. Yes, here it's more about general good practice.

    Imagine that tomorrow a security flaw is discovered in current PW (2.7.2 and older). So, the day after tomorrow :-) , PW 2.7.3 is released, fixing the flaw. So everyone upgrades to 2.7.3. But if some site is not upgraded, then it is at risk. And, as PW discloses its version number, the attackers will be happy :-)

  13. Hi, I'm happily contributing to populating the security section :-) .

    First, let me say that I highly regard Processwire for its security framework and for its security-conscious core implementation.

    Now, my issue : the fact that get() requests are not subject to access control, because, as stated in the selectors documentation, "if you are asking for a single specific page, we assume you mean it".

    -> I must say that, everytime I read this sentence, my jaw drops :-) ... what ? PW does never make any asumption about our frontend code, but *here*, on a very sentitive *security* subject, PW does the *asumption* that my code does not need the security framework and as a result it is bypassed ?!.... the reality is, that yes I can easily imagine situations where I would not be sure that the request would target an allowed page, especially if the workflow would involve prior input from the frontend users.

    Ok, you can always circumvent this, by enforcing your own controls, and/or you can write your code differently in order to forbid any absolute reference to pages to reach the frontend users, but, 

       .1. first, it's actually the aim of the CMF security framework, to give you tools which make your life easier (and less risky) when dealing with security aspects. Personnally I like to use PW access control in my design of the site resources access policy, but because of this get() behaviour I now have become very cautious EACH time I write a get(....) , thinking twice about "could it be subject to an breach scenario ?" ... and for safety, I sometimes switch to using a find() instead.

       .2. even in PW core, there's frontend code which makes use of absolute reference to pages : in comments workflow

    -> Personally, I would feel much better if, for instance, there was a "$config->accesscontrol_get = true" setting which would allow to enforce access control also on get() requests ; plus the ability to override this behaviour for a given request, just like find().

    As a side note, related to absolute references to pages ; there is inherently another risk, because everything in PW is a page, so, when the frontend user has access to a single occurrence of this, it allows him to try to grab the whole site data (then comes the access control into play).

    For security in depth, I really don't like this, and, in my code, everywhere this would happen, I have done things differently (I even applied my method & patched the standard comments workflow :-) . This was subject to following thread : https://processwire.com/talk/topic/10624-more-security-in-regards-to-page-id/

    thanks

    Guillaume

    • Like 2
  14. Thanks a lot, this menu system is really performing well.

    I think there's a small mistake at the end of the template code ; it is auto-fixed by most browsers, but not by all. The very last block should be :
     

              function(Page $page) {
                  if ($page->numChildren > 0) {
                      echo '</ul>';
                  }
                  echo '</li>';
              }
    
    • Like 1
  15. Recursive hanna codes is mentioned already a few time in this topic. There's an open pull request on the github repo, that adds support for that, but the official release doesn't have that right now.

    +1 for the bug with recursive/nested hanna code textformatters, +1 for the github pull request which does fix the issue, and +1 for having it integrated to the official release

    cheers

  16. Hello, 
     
    Here is a security related feature request.
     
    I am having more and more use of $page->id as a GET or POST parameter, for various workflows in frontend site.
    Processwire itself is making use of it at some places related to frontend, eg. for comments submission workflow.
     
    My problem is : This is an absolute AND predictable value : from 0 to N. So, when used for submission by the users, it allows a malicious user to forge requests in order to perform a FULL crawling of the website pages. Even pages that are otherwise not accessible by following the website links. Of course, Processwire access permissions apply ; but then, any site-specific permission weakness will result in information disclosure. Overall, this is not very satisfying.
     
    What would be best, instead, is the ability to make use of an absolute AND NOT predictable value : a $page->encodedId (build with something like http://hashids.org/php/)
    Along with a commodity method getDecodedId(), for retreiving the associated $page->id.
     
    Fact is, I am doing something similar to this in the templates which need it. 
    And, for easier usage, I plan to generalize this to all templates, with some coding which implies hooks on template creation & on page creation, for automatically adding a $page->encodedId field at each template creation + automatically populating its value at each page creation/cloning.
     
    But before I go into this, I would like to submit this feature request : I would rather have this in Processwire core :-)
    Processwire itself would directly benefit from this feature (see comments submission workflow, for instance).
     
    I hope it makes sense for someone else than me :-)
    cheers

    • Like 3
  17. Hello, 

    In a template, I cycle through all site languages, 

    and, for each language, 

     - I temporarily set $user->language to this language

     - I call $pages->get("/anotherpage/")->render()

    in order to render this same content in all languages

    Problem : 1st call to render() is ok, but on second call, error occurs : 

    Compile Error:  Cannot redeclare functionxyz()

    A few additional tests show that actually, render() kind of includes the called template code within the calling template code, and as a result, if any function name is already declared, this error occurs obviously.

    Here, in my case, it's with the 2nd call to the called template that this occurs.

    So, just to make sure, what's the best coding practice regarding this behavior ? Is there an API solution for avoiding the problem ?

    Or should I adapt the code of the called template ? (by surrounding each function with "if(!function_exists" , or by putting all its functions in another file and include_once it)

    thanks

  18. I am facing a strange issue with lazycron,

    maybe I don't use it properly.

    It is called within the main template, and it is set to trigger every 30 minutes :

    wire()->addHook('LazyCron::every30Minutes', null, 'publish_new_stuff');

    For debugging purposes, the triggered function starts with logging inside a text file :

    function publish_new_stuff(HookEvent $e)
    {
    $log = new FileLog(wire('config')->paths->logs . 'my-log2.txt');
    $log->save('coucou inside lazycron');
    ...

    What I observe is :

    - it used to work

    - then since a few days it is not triggered anymore (the log file is not updated)

    - as soon as I change the time interval to everyMinute, every2Minutes, or every5Minutes, then it is working again

    - as soon as I change the time interval to every10Minutes, every30Minutes, every45Minutes, or everyHour, then it STOPS working

    - I have uninstalled/reinstalled the LazyCron module from the backend, no change

    any idea ?

    what can I do to further troubleshoot or fix ?

    thanks

×
×
  • Create New...