Skip to content

gh-146311: Reject non-canonical padding bits in base32, 64, & 85 decoding - #146312

Merged
gpshead merged 17 commits into
python:mainfrom
gpshead:gh-146311-nonzero-padding-bits
Apr 25, 2026
Merged

gh-146311: Reject non-canonical padding bits in base32, 64, & 85 decoding#146312
gpshead merged 17 commits into
python:mainfrom
gpshead:gh-146311-nonzero-padding-bits

Conversation

@gpshead

@gpsheadgpshead commented Mar 22, 2026

Copy link
Copy Markdown
Member

Summary

Add canonical=False keyword argument to a2b_base64, a2b_base32, a2b_base85, and a2b_ascii85 (and their base64 module wrappers). When canonical=True, non-canonical encodings are rejected per RFC 4648 section 3.5.

This is independent of strict_mode.

For base85/ascii85, the check also rejects single-character final groups (never produced by a conforming encoder) and verifies partial group padding matches what the encoder would produce.

gpsheadand others added 2 commits March 22, 2026 15:05
RFC 4648 section 3.5 allows decoders to reject encoded data containing
non-zero pad bits. Both a2b_base64 (strict_mode=True) and a2b_base32
currently silently discard non-zero trailing bits instead of raising
binascii.Error. These tests document the expected behavior.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add leftchar validation after the main decode loop in a2b_base64
(strict_mode only) and a2b_base32 (always). Fix existing test data
that incidentally had non-zero padding bits to use characters with
zero trailing bits while preserving the same decoded output.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gpshead
gpsheadforce-pushed the gh-146311-nonzero-padding-bits branch from 8451e22 to 0ca2563CompareMarch 22, 2026 22:07
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gpshead

Copy link
Copy Markdown
MemberAuthor

discussing if base32 needs strict_mode on the issue. not adding a NEWS entry until that is decided.

@gpsheadgpshead self-assigned this Mar 22, 2026
gpsheadand others added 3 commits April 4, 2026 22:40
Gate non-zero padding bits rejection behind a new canonical= keyword
argument independent of strict_mode, per discussion on pythongh-146311.
Per RFC 4648 section 3.5 ("Canonical Encoding"), decoders MAY reject
encodings where pad bits are not zero. The new canonical=True flag
enables this check for a2b_base64, a2b_base32, a2b_base85, and
a2b_ascii85.
For base85/ascii85, the canonical check also rejects single-character
final groups (never produced by a conforming encoder) and verifies
that partial group encodings match what the encoder would produce.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The _Py_ID(canonical) identifier used by the clinic-generated
argument parsing code needs to be registered in the global strings.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gpsheadgpshead changed the title gh-146311: Reject non-zero padding bits in base64/base32 decodinggh-146311: Reject non-canonical padding bits in base32, 64, & 85 decodingApr 4, 2026
gpsheadand others added 4 commits April 4, 2026 23:51
RFC 4648 only covers base16, base32, and base64. The canonical
encoding concept applies to base85 but is not defined by that RFC.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the re-encode-and-compare loops with a quotient comparison:
two divisions by 85**n_pad tell us whether the decoded uint32 and
the zero-padded output bytes share the same leading base-85 digits.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Test non-canonical rejection for all partial group sizes (2/3/4 chars)
- Test digit-0 1-char group for ascii85 (exercises chunk_len==0 guard)
- Test boundary byte values (\x00, \xff) at each group size
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Round-trip tests: encoder always produces canonical output (base64,
base32, base85, ascii85)
- Uniqueness tests: for base85/ascii85 partial groups, sweep all 85
last-digit values and verify exactly one decodes to the original
payload with canonical=True
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@serhiy-storchakaserhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM technically, but I have two notes:

  • The check for single-character final group should be unconditional, like in other codecs. This is the part of the specification, without MAY.
  • "Canonical" Ascii85/Base85 encoding is not defined, this is a projection. And in case of Ascii85 many other deviations of "canonical" encoding are accepted by default and not checked. As minimum, we should use "canonical" in quotes here, or don't use that word for these encodings.

Comment threadModules/binascii.c Outdated
Comment threadModules/binascii.c Outdated
gpsheadand others added 4 commits April 5, 2026 18:48
Per the PLRM spec (section 3.13.3), a final partial 5-tuple containing
only one character is an encoding violation. Move this check outside
the `canonical=` guard so it is always enforced.
Also change chunk_len and i from Py_ssize_t to int per review feedback.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When canonical=True, reject '!!!!!' (five zero digits) in favor of
the 'z' abbreviation. The PLRM spec defines 'z' as the representation
for all-zero groups, so '!!!!!' is a non-canonical encoding.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Document that single-character final groups are always rejected
- Add versionchanged:: next markers for the behavioral change
- Update canonical= description for ascii85 to mention z-abbreviation
- Update canonical= description for base85
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment threadDoc/library/base64.rst Outdated
Comment threadDoc/library/binascii.rst Outdated
Comment threadDoc/library/binascii.rst Outdated
@serhiy-storchaka

Copy link
Copy Markdown
Member

discussing if base32 needs strict_mode on the issue. not adding a NEWS entry until that is decided.

It was decided to use canonical, right?

Please update also What's New.

…ro-padding-bits
# Conflicts:
#	Doc/library/base64.rst
…atsnew
- Reorder versionchanged param lists alphabetically in base64.rst and
binascii.rst (canonical, ignorechars, padded).
- Restore "two to five" in a2b_ascii85/a2b_base85 partial-group text;
partial finals are 2-4 chars but a complete final group is 5.
- Drop redundant versionchanged blocks from a2b_ascii85/a2b_base85
since both functions are new in 3.15.
- Add Misc/NEWS.d entry and Doc/whatsnew/3.15.rst bullets covering the
new canonical kwarg and the unconditional 1-char rejection.
The PLRM (Adobe Ascii85) and the various base85 alphabets do not define
a "canonical" form. Where the underlying spec doesn't ground the term,
the doc text now says explicitly that "canonical" here means "the
encoding the corresponding b2a_* function would produce."
Base16/32/64 entries are unchanged: RFC 4648 grounds the term there.
@gpshead

Copy link
Copy Markdown
MemberAuthor

It was decided to use canonical, right?

yeah given our discussion on the issue that seemed like the best name. I also updated the *85 API docs to cover what "canonical" specifically means to us given that isn't defined by any of the looser specs for those.

@gpshead
gpshead marked this pull request as ready for review April 25, 2026 19:15
@gpshead
gpshead requested a review from AA-Turner as a code ownerApril 25, 2026 19:15

@serhiy-storchakaserhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

@gpshead
gpshead merged commit c5fcdb4 into python:mainApr 25, 2026
60 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@gpshead@serhiy-storchaka