Excess data in not handled properly in binascii.a2b_base64() #87252

Activity

1. idan22moral commented on Jan 31, 2021

   

   idan22moralmannequin

   on Jan 31, 2021

   MannequinAuthor

   More actions

   Currently, when providing binascii.a2b_base64() base-64 input with excess data after the padding ('='/'=='), the excess data is ignored.

   Example:

   import binascii
   binascii.a2b_base64(b'aGVsbG8=')       # b'hello' (valid)
   binascii.a2b_base64(b'aGVsbG8==')      # b'hello' (ignoring data)
   binascii.a2b_base64(b'aGVsbG8=python') # b'hello' (ignoring data)

   Note: MANY libraries (such as the all-time favorite base64) use this function as their decoder.

   Why is it problematic:

   • User input can contain additional data after base64 data, which can lead to unintended behavior in products.
   • Well-crafted user input can be used to bypass conditions in code (example in the referenced tweet).
   • Can be used to target vulnerable libraries and bypass authentication mechanism such as JWT (potentially).

   The logic behind my fix PR on GitHub:

   • Before deciding to finish the function (after knowing the fact that we passed the data padding),
     we should check if there's no more data after the padding.
   • If excess data exists, we should raise an error, free the allocated writer, and return null.
   • Else, everything's fine, and we can proceed to the function's end as previously.

   Though not publicly disclosed, this behavior can lead to security issues in heavily-used projects.
   Preventing this behavior sounds more beneficial than harmful, since there's no known good usage for this behavior.

   From what I read, the python implementation in not so close (when speaking about this case of course) to the base64 RFC.
   (link: https://tools.ietf.org/html/rfc4648#section-3.3)

   Thanks to Ori Damari (twitter: https://twitter.com/0xrepnz) for bringing this behavior up,
   and thanks to Ryan Mast (twitter: https://twitter.com/rmast), and many of the other great guys for discussing the problem in the comments.

   Link to the tweet: https://twitter.com/0xrepnz/status/1355295649915404291

   --------------------------

   Idan Moral
   Twitter: https://twitter.com/idan_moral
   GitHub: https://github.com/idan22moral
2. idan22moral

   added

   3.10 (EOL)end of lifeend of life

   stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory

   type-securityA security issueA security issue

   on Jan 31, 2021
3. gpshead

   self-assigned this

   on Mar 13, 2021
4. gpshead commented on Jul 19, 2021

   

   gpshead

   on Jul 19, 2021

   Member

   More actions

   New changeset 35b98e3 by Idan Moral in branch 'main':
   bpo-43086: Add handling for out-of-spec data in a2b_base64 (GH-24402)
   35b98e3
5. gpshead commented on Jul 19, 2021

   

   gpshead

   on Jul 19, 2021

   Member

   More actions

   I've merged Idan's PR adding a strict_mode parameter to a2b_base64. It defaults to False for backwards compatibility.

   From a security perspective, it'd be _ideal_ if this were True. But I expect doing that would break a bunch of existing code and tests that has been relying on some of the former leniency behaviors so I recommended the conservative approach of the old-behavior default. It'd be a good thing to change it to True, but disruptive. We need motivating reason to do that.

   As it is a new feature due to the new parameter, this is for 3.11.

   Workaround for Pythons without this: do a validity check before calling a2b_base64. I suspect a regex could be constructed for that if you're careful. If you come up with one, please share it here.
6. gpshead

   added

   extension-modulesC modules in the Modules dirC modules in the Modules dir

   3.11only security fixesonly security fixes

   and removed

   3.10 (EOL)end of lifeend of life

   on Jul 19, 2021
7. gpshead

   closed this as completedon Jul 19, 2021
8. ezio-melotti

   transferred this issue fromon Apr 10, 2022