The original issue is still stuck on 6.x, so splitting off the 8.x issue here #2380053: [core] Posting an array as value of a form element is allowed even when a string is expected (and bypasses #maxlength constraints) - first step: text fields
Transferring this issue from security.drupal.org
The Security Team considered including this in SA-2014-06, but after debate about unexpected side effects decide to have it as a public issue and commit the prepared patch soon after the SA.
catch agreed that this could be committed to 7.x and 6.x in advance of an 8.x patch, as if it had been part of the SA. That hasn't happened yet for 6.x, so this patch is to forward port the 7.x fix to 8.x
Commit credit: klausi, pwolanin, tsphethean, sun
posting an array as value of a required and maxlength limited textfield will cause the field to pass both the #required => TRUE and #maxlength validation.
Enforce the expected data type in the Form API
Forward port 7.x fix to 8
n/a
FAPI change to enforce correct data type of certain fields
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | interdiff.txt | 8.27 KB | dawehner |
| #4 | 2560137-4.patch | 13.45 KB | dawehner |
| #2 | 2560137-2.patch | 5.17 KB | pwolanin |
Comments
Comment #2
pwolanin commentedFirst pass at forward port. Needs the tests ported also.
NR just for the bot.
Comment #3
tim.plunkettAll of this could be unit tested.
Is this really a release blocker?
Comment #4
dawehnerIts just fun to write them.
Given that the d6 and d7 issue has been critical I think this is also critical. This is also a security improvement ...
Comment #5
catchSo this is the first time we've skimped on the backport policy during nearly the entire 8.x cycle and it very nearly went horribly wrong. Just posting this as a marker for the next time that discussion comes up.
@pwolanin thanks for digging this one out!
Comment #6
catchAlso tests here look great, the password confirm code we could probably PHP 5.4-ify but I think we should get the straight forward-port in as is.
Generally security hardening is major, but had we fixed this before SA-CORE-2014-005 was discovered it would have severely mitigated the attack vectors for that and potentially lengthened the couple of hours window before sites got hacked in the wild. The only reason this wasn't included in that release was to avoid holding up the SQL injection fix going out asap.
RTBC for me.
Comment #7
alexpottThis is really nice security hardening. Hopefully in d9 we'll get to a place where FAPI ensures the type matches what you would expect but this is a great start and not disruptive at all.
Committed e513467 and pushed to 8.0.x. Thanks!