IOS 6 injecting should_group_accessibility_children to POST requests
logicalfriday.com
logicalfriday.com
1. They were blindly throwing the entire set of POST data into a Rails model.
2. This started throwing errors when they got an unexpected key in POST.
3. They "solved" the problem by writing a middleware to filter out that specific key from POST.
I don't even know where to begin.
Watch next time Apple alter some http headers and break your API.
Which meant that when a new property showed up their app blindly submitted it to their web API, and their web API blindly accepted it because it was doing mass assignment, and that's when the API broke.
Which really just hammers home the point people have been trying to get you to see, which is that these types of idioms -- mass assignment, blind trust of client-supplied data, blacklisting instead of whitelisting -- are really serious problems that should not be encouraged, and should not be swept under the rug.
If there was a parameter that gave a user admin access, then Rails might accept such a parameter and that might be used to take control of the app.
It's similar to what happened to Github.
https://github.com/rails/rails/issues/5228
I would think that the API would validate the POST parameters, ignore unexpected parameters and give errors for malformed expected ones. Taking it a bit further the developer should then be notified of malformed POST parameters being present and decide if it is a bug or an attack.
Instead of modifying at the middleware or persistence layers, copy relevant fields out of the POST hash into an intermediate hash. Which, generally, will be more secure anyways.
From a security and usability perspective, the programmer must always be in ultimate control of what code is being executed. If it's "parameters", an API should be explicitly requesting any or all parameters, and completely ignoring unknowns. If the API is scanning for unknown parameters or even worse scanning for parameters to invoke variable named function calls (it happens a lot), at best; your API is wide open for attack. (This was a major attack vector in some Wordpress APIs in the recent past.) In the meantime, the unknown parameter just causes the program to break, and why would any programmer allow that if it's so easily preventable.
UPDATE: editing to reply since hn won't let me reply directly because the thread is to deep, yet I'm getting downvotes
It's not a tautology. Some things are safe even if you do them wrong. Some things are unsafe no matter how you do them.
Rails changed the defaults so that now you have to deliberately decide to do things unsafely. Rails before 3.2.3 fails un-safe in this scenario, but later versions fail safe. Rails 4 uses a different solution that's even harder to screw up.
That's a tautology.
In general you can't count on code being written "correctly", so this isn't a defense. It is better to have systems that degrade gracefully in the face of humans and their idiosyncrasies, rather than those that fail-unsafe, because you can't build your security system on the assumption that your code will be written by superhumans.
Users of a framework should have to go out of their way to make themselves insecure. It shouldn't be insecure by default.
Not exactly. Most web frameworks don't have a built-in "mass assignment", let alone enable it by default.
update to reply because of downvotes:
1) butterfly knives are very useful tools
2) mass assignment can be used safely out of the box in rails post v3.2.3. To use it, you have to explicitly add parameters to the whitelist or disable the whitelist. The article is there to explain why disabling the whitelist is a bad idea.
Edit to reply to edits: Mass assignment is still dangerous "out of the box" since you have to switch on the whitelist behavior by calling attr_accessible on your model classes. In the security guide, the older, more dangerous, attr_protected is introduced first.
I think every rails dev should be familiar with the security guide, but more than that I wish that security was the default. While anybody is free to make an app as insecure as they wish, it should be the exception rather than the default.
Mass assignment is a fairly normal thing to do, any parameters you want the model to ignore can be marked as so in the model and then mass assignment will work anywhere. Rails loves DRY.
Since the only way in which one can get wrong arguments is if somebody is hacking you (or attempting to do so) in which case just throwing an error at them is a fair thing to do.
Because that seems entirely the wrong way around.
EDIT: looked at the link posted elsewhere here[1] and ound that it is possible to whitelist using "attr_accessible". Please tell me people know about and use this.
[1] http://guides.rubyonrails.org/security.html#mass-assignment
Rails gives you attr_accessible and attr_protected to alleviate this problem. Protip: the one from those two that you suggested to use is just as terrible an idea as allowing blind POST into your database. Blacklisting is always the wrong approach to security. I don't care if you have to repeat yourself and type more characters: use the whitelisting of attr_accessible.
The real culprit is Apple. It's just not ok to add unexpected parameters to people's POSTs. If anything, they should have put it in an HTTP header instead.
I, for one, don't like having my application error logs polluted by script kiddies running against my wall...
No, it's an abnormal thing to do and a security hole.
> any parameters you want the model to ignore can be marked as so
Which is complete insanity.
> Rails loves DRY.
Yes so does PHP, hence register_globals.
That's on the "idiotic" side of "I don't want to write stuff", and even the Rails staff stopped defending this insanity back in March.
And register_globals was widely used in PHP, that does not make it anything but moronic.
for (var prop in object)
http://developer.apple.com/library/ios/documentation/uikit/r...
A good short-term fix is to update the web service, but they should update it to only use the POST parameters it expects. When they release an update to the iOS app, they can fix their serializer to only serialize the properties they need.
Apple isn't injecting this - their code is.
If you do something like:
@user = User.new(params[:user].slice(:name))
It will be safer, and avoid the problem.
It's weird that iOS is doing that... but (at the risk of piling on) browsers do all sorts of weird stuff. Your app needs to deal with it.
For non-Rails programmers an idea such as mass assignment sounds very strange, but if you ignore that, it still sounds odd at first request than an OS would inject parameters into a HTTP post request.
I have inspected several POST requests from iOS 6 apps and have not seen anything like this yet.
Yes.
Any webapp that I would consider secure MUST validate all input from clients. This includes white listing any and all parameters names, preferably in middleware, but at least in the controller. Allowing random keys in your input seems recipe for disaster, when you consider a multi layer app security policy. While this may seem like an overkill to some, this is best practice I've seen implemented in any project that deals with real money.
Hence, such a change in iOS WILL break any such application. Irrespective of your views on the sanity of mass assignment.
I would like to see a discussion on why apple thought it would be a good idea to introduce a new parameter to every request. Any ideas?
Mass-assignment and similar patterns (of shoving all request parameters into your trusted content, see also `extract` and `register_globals`) "allow random keys into your input", ignoring said keys doesn't.