Blacklist your model fields for save()…

You’ve probably heard that in order to make your save() more secure, you can pass-in a third parameter of only those fields that you’d like to save (all others will be ignored).

However, in some cases it would make more sense to “blacklist” one or two fields, rather than “whitelist” a whole bunch of required fields.

An awesome tip from Nate, shows just how to accomplish this…

$blackList = array('protected', 'fields', 'here');
$this->Model->save($this->data, true, array_diff(array_keys($this-> Model->schema()), $blackList));

Quite clever indeed.

m4s0n501
  • http://identoo.com/dirk.olbertz Dirk Olbertz

    Blacklisting is always bad, as you need to go through all your code once you add a field in the table.

    I rather would go for the extra lines to have the actual fields whitelisted.

  • Sharkoon

    Wow nice! I didnt even know that allowed field can be passed :)

  • http://teknoid.wordpress.com teknoid

    @Dirk Olbertz

    I have to disagree with “always bad”, let’s say in a given Model I never want to save ‘id’, because I know it should never be updated… are you saying it makes more sense to white-list all other fields instead? Wouldn’t that be exactly opposite of what you’d like to avoid if now I add a new field to that table?… Either way, having both options in such an easy way is quite nice to have.

    @Sharkoon

    Killing two birds with one stone ;)

  • http://identoo.com/dirk.olbertz Dirk Olbertz

    The “bad” thing is, that whenever you add a new field to the table, you need to go through all your code to find the places where you save to this table in order to adjust your blacklist. Otherwise you risk data loss, because someone just passes additional data to the POST request of your controller.

    When doing whitelisting, you will have no risk of unintended data loss – you just would have to check your code to perform the new intended behaviour of saving this field.

    Therefore I rather go with whitelisting, because I never risk that a field may be overwritten, although I did not authorize it.

    You can reduce this problem by using the security methods for the form helper, but this will not help, if you have an API for your application and need to handle the incoming data yourself.

    And regarding $this->id: I recommend to check the CakePHP code on this particular matter, because there was a problem at least in one of the 1.2 betas. You need to be very carefull on how to set the id. And remember: never put it in the form itself, or otherwise people will be able to overwrite data that does not belong to them.

    Handling user input is a very complex matter and therefore I rather go with “never blacklisting” to reduce the complexity of that problem for me.

  • http://teknoid.wordpress.com teknoid

    @Dirk Olbertz

    I think you have it backwards. Imagine I have a ‘counter’ field in my table, I know that I never want to save this field, because it is calculated. So I blacklist this one field…
    Now, it doesn’t matter how many other fields I add to this table, I need not do anything else in the code as ‘counter’ will never be saved, and that’s all I care about for this given model.
    If I was to whitelist all the allowed fields, then that would require me to go into each save() method and add new fields to the list, if I added new columns to the table.

    That being said, it is a good idea to keep a list of allowed fields in the model and do something like $this->ModelName->allowedFieldsForSave; so whether you are whitelisting or blacklisting you only update the list in one place.

    Regarding the ‘id’, you can use the Security component to prevent anyone tampering with your forms (actually it’s always a good idea to use it). It’s not always an option to keep id in the session, since you might allow data from external sources, while no session is established.

  • Flexer

    How about going one step further and adding a column in your DB schema which act as a flag for blacklisted/whitelisted fields and then having a simple Behavior to take these into account ?

    G.

  • http://identoo.com/dirk.olbertz Dirk Olbertz

    I’m more concerned about security than convenience. When whitelisting and I add a field to the table, I normally do this for exactly one form and therefore only have to change the field list in the save() for one controller method.

    But when blacklisting, I need to make sure to get to all methods where this model is saved/updated.

    I guess it’s a taste issue and therefore non.negotiable :-)

  • http://teknoid.wordpress.com teknoid

    @Flexer

    Is that really necessary? What would be the benefit of having to rely on extra DB flag?

    @Dirk Olbertz

    Fair enough. We’ll agree to disagree ;)

  • Martin Westin

    I remember when the whitelist vs blacklist was up for discussion a short while ago. Nate’s trick is definitely one of those “well of-course, why didn’t I think of that” sort of things.

    When tampering was big issue on one project I was in the fortunate situation of being able to lock each user into their own database and thus eliminating many cross-user problems that way. This would of-course not work for Facebook or Twitter so the preference may in part depend on the type of application you work on.

  • http://www.webiscake.com Jimmy Bourassa

    @Dirk Olbertz :

    Regarding “never showing the id in the form”, there’s a pretty safe workaround for that. If you use the Auth, you just have to make sure user x has the requiered authorization to perform y action with z parameter(s). All of this is made very simple and structured with the Auth component. And there are no security drawback that I am aware of.

  • http://www.mysiteonline.org/ Brendon Kozlowski

    Very nice tip, teknoid – also glad to see an update, glad you’re back. I actually ventured in to IRC a couple times to see if you were around due to lack of updates. ;)

  • http://teknoid.wordpress.com teknoid

    @Martin Westin

    Yeah, I pretty much slapped myself in the head as well :)

    @Jimmy Bourassa

    Combine that with the Security component and your forms are bullet-proof (at least as far as your app goes).

    @Brendon Kozlowski

    I know, I’ve been MIA for a while… just work and travel in the last months have kept me very busy, hopefully I’ll have some great things to blog about very soon ;) Stay tuned…

    I don’t even have time to hang out on my favorite IRC channel :(

  • keymaster

    Why not just unset() the blacklisted fields before saving?

  • http://www.steveoliveira.com Steve

    Dirk is right when it comes to security. It’s better to have deny all first, then allow specifics second (which is what whitelist does). However, some models don’t need that kind of security and in matters of convenience, blacklist will do. On that point, I also agree with teknoid that blacklisting is not “always” bad

  • http://teknoid.wordpress.com teknoid

    @keymaster

    This is just as simple, IMO… and looks nicer :)

    @Steve

    IMO, it really depends on one’s specific need, and the most important part is not to forget to use the Security component ;)

  • http://www.steveoliveira.com Steve

    Hey teknoid, sorry to derail the discussion but what do you mean by “work and travel”. Are you traveling as part of your work or just traveling for personal reasons and working as well?

    Just curious, because I plan on doing the latter

  • http://teknoid.wordpress.com teknoid

    @Steve

    I took time off to travel (personal, much needed time), then I was freelancing quite a bit, and now I started working full-time again.
    And with all of that happening, kind of, one after another (or at the same time :)) things just have been a bit hectic, but hopefully I’ve now settled down a little…

  • Pingback: CakePHP : signets remarquables du 08/03/2009 au 12/03/2009 | Cherry on the...()

  • brian

    Cool tip, but I must be doing something wrong. I tried a whitelist (only save these fields), but the fields in my schema that are “not null” still get included in my update statement,and I don’t know why. I have tried passing in $this->data that only includes fields to be save and $this->data that has values for the whole schema.

    Anyone have an idea what I’m doing wrong, why my fields defined as “not null” still get added to update statement? I think these fields get added in this code in save()
    Behaviors->trigger($this, ‘beforeSave’, array($options), array(
    ‘break’ => true, ‘breakOn’ => false
    ));
    if (!$result || !$this->beforeSave($options)) {
    $this->whitelist = $_whitelist;
    return false;
    }
    }
    ?>

  • http://www.mysiteonline.org/ Brendon Kozlowski

    @teknoid: Your own personal time takes precedence, no question about that, I’m just glad everything’s OK – you’re not deathly ill, sick, in a car wreck, etc… ;) For your own sanity, I hope things will settle down as you’re expecting them to and you can have some more time available to you.

    Thanks again for informing us of the blacklist tip. :)

  • http://teknoid.wordpress.com teknoid

    @Brendon Kozlowski

    Thanks :) … and really hats off to Nate on this one ;)

    @brian

    Hard to tell. Those fields don’t have any default values by chance?
    This might be a good question to ask at the mailing list…

  • Pingback: cakealot()

  • Pingback: Don’t change the meaning of parameters - cakebaker()

  • jippignu

    Hey,

    my 50 cent – “Whitelistable” behavior:

    http://bin.cakephp.org/view/1251115625 sample code. use it in most of my apps :)

  • http://teknoid.wordpress.com teknoid

    @jippignu

    Looks interesting, thanks for sharing ;)

  • brian

    @teknoid: thanks, you are right, I should ask in the list. Your articles are great, by the way, learned a lot from your site.

  • http://teknoid.wordpress.com teknoid

    @brian

    Good to hear ;) Glad they helped out.

  • Pingback: CakePHP Digest #10 - News Overload | PseudoCoder.com()

  • Pingback: Cakephp important facts you must know - Part 1 | DevArticles.In()

  • Pingback: Don’t change the meaning of parameters | linkfeedr()

  • Pingback: Blacklist your model fields for save()… | Dailytuts.net - Daily tutorial for peoples()