Skip to content

Return early from OAEP Unpad on undersized blocks - #1374

Open
Coralesoft wants to merge 1 commit into
weidai11:masterfrom
Coralesoft:fix/issue-1370-oaep-undersized-block
Open

Coralesoft wants to merge 1 commit into
weidai11:masterfrom
Coralesoft:fix/issue-1370-oaep-undersized-block

Conversation

@Coralesoft

Copy link
Copy Markdown
Contributor

Refs #1370, follow-up to 3269c25.

OAEP_Base::Unpad marks undersized blocks invalid but continues through masking and digest verification before returning. The saturating subtraction prevents dbLen from wrapping, but these later accesses can still read past the block.

Reproduced under AddressSanitizer through the public TF_DecryptorBase::Decrypt path:

  • RSA-512/SHA-512 gives a 63-byte internal block, shorter than the 64-byte seed. While unmasking the seed, a 16-byte read in xorbuf crosses the allocation boundary by one byte.
  • RSA-1024/SHA-512 gives a 127-byte internal block. After the 64-byte seed, only 63 bytes remain for the 64-byte digest. An 8-byte read in VerifyBufsEqual crosses the allocation boundary by one byte.

Unpad now returns an invalid decoding when the internal block is smaller than 2*hLen+1, before setting up the seed and DB buffers or masking. The check depends only on the block length and hash choice.

Pad applies the matching bound and checks the input length by subtraction to avoid overflow. The assertion follows the runtime check so invalid sizes throw in debug builds too.

Verified locally with GCC 13 and -O1 -fsanitize=address: both decrypt cases return an invalid decoding with no sanitizer reports after the change. cryptest v passes with no sanitizer reports, and cryptest tv rsa_oaep passes all tests.

OAEP_Base::Unpad set the invalid flag for a block too small to hold an encoding but continued to lay it out, reading past the block before the flag was checked. Return before the layout instead.

Tighten the Pad bound to the full 2*hLen+1 minimum and write the input-length check as a subtraction so a large inputLength cannot overflow it. Move the invariant assertion after the throw so debug and release builds agree.
Sign up for free to 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.

1 participant