fix: restore alloc-size >= ivlen invariant in legacy AES-GCM SET_IVLEN - #3466
Open
dougch wants to merge 1 commit into
Open
fix: restore alloc-size >= ivlen invariant in legacy AES-GCM SET_IVLEN#3466dougch wants to merge 1 commit into
dougch wants to merge 1 commit into
Conversation
Contributor
|
🔒 Security Review — View Report Please review before merging. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3466 +/- ##
==========================================
+ Coverage 78.11% 78.14% +0.02%
==========================================
Files 700 700
Lines 125599 125628 +29
Branches 17372 17378 +6
==========================================
+ Hits 98117 98177 +60
+ Misses 26611 26580 -31
Partials 871 871 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Port BoringSSL 04aa32f96. EVP_CTRL_AEAD_SET_IVLEN's old guard (arg > EVP_MAX_IV_LENGTH && arg > gctx->ivlen) let gctx->iv keep pointing at an undersized heap buffer after a COPY-then-grow sequence, bypassing the ivlen >= 8 guard added for AWS-LC-1298. Adds TEST(CipherTest, GCMRepeatedlyChangeIVLength) mirroring upstream's coverage. Regression manifests as OOB heap access; Linux ASan CI catches it.
justsmth
force-pushed
the
port/boringssl-aes-gcm-set-ivlen
branch
from
September 8, 2026 21:22
a0ee5b5 to
d86460a
Compare
justsmth
approved these changes
Sep 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context and motivation
EVP_CTRL_AEAD_SET_IVLENdid not maintain the invariant thatalloc-size(gctx->iv) >= gctx->ivlen. Its old guardarg > EVP_MAX_IV_LENGTH && arg > gctx->ivlenskipped reallocationwhen shrinking (leaving
gctx->ivpointing at a stale heap bufferwhile callers assumed the built-in
c->ivwas in use) and, after anintervening
EVP_CIPHER_CTX_copy, allowed a subsequent grow withinthe "no reallocation" range to raise
gctx->ivlenabove the actualbuffer size. This bypasses the
ivlen >= 8guard onEVP_CTRL_GCM_IV_GENand lets its 8-byte counter store landout-of-bounds.
Concrete bypass sequence, all through public EVP APIs:
SET_IVLEN(large)allocates a heap bufferSET_IVLEN(4)leavesgctx->ivpointing at that heap bufferbut sets
ivlen = 4EVP_CIPHER_CTX_copytriggersEVP_CTRL_COPY, which doesmemdup(gctx->iv, gctx->ivlen)= a 4-byte heap allocation onthe destination
SET_IVLEN(8)on the destination fails the old guard(
8 > 16 && 8 > 4→ false), so no reallocation, and nowivlen = 8sits on a 4-byte bufferEVP_CTRL_GCM_IV_GENpasses itsivlen < 8check, then writes8 bytes at
iv + ivlen - 8 = iv + 0, four of which are OOBDescription of changes
Ports BoringSSL commit
04aa32f9.EVP_CTRL_AEAD_SET_IVLENis rewritten into two explicit branches:arg <= EVP_MAX_IV_LENGTH— ifgctx->ivcurrently points at aheap buffer, free it and reset
gctx->iv = c->iv. This makes thesmall-IV state always live in the built-in buffer.
arg > gctx->ivlen— allocate the new buffer first, then free theold one, so an OOM leaves the context intact.
in place.
With the invariant restored, the existing
EVP_CTRL_COPYhandler'smemdup(gctx->iv, gctx->ivlen)reads a validly-sized buffer in everyreachable state, so no separate change to the copy handler is needed.
Testing
Adds
TEST(CipherTest, GCMRepeatedlyChangeIVLength), mirroring theshape of upstream's regression test. Six length sequences (monotonic
grow and shrink in both the heap and the built-in-buffer regime, plus
two zig-zag sequences crossing
EVP_MAX_IV_LENGTH) run in bothno-copy and copy-interleaved variants, comparing the resulting
ciphertext and tag against a straight-line reference encryption.
The regression manifests as an OOB heap access, not as a ciphertext
mismatch.
aes_gcm_init_keycomputes GCM state from the caller'sivpointer, not fromgctx->iv, so the specificEncryptUpdate/GET_TAGpath this test drives produces the correct ciphertext evenwhen
gctx->ivis undersized — the OOBmemcpyintogctx->ivsmashes adjacent heap silently. This test therefore only fails
pre-fix under an AddressSanitizer build. Please pay particular
attention to the Linux ASan CI job; that is the signal that
distinguishes fixed from broken.
Review considerations
crypto/fipsmodule/cipher/e_aes.cis inside theFIPS module. This change is internal buffer-management bookkeeping
only. No public API, no ABI change, no algorithm behavior change
visible outside the module, no new entry points. The bypass path
was reachable before this change; it is not any more.
ivlen < 8inEVP_CTRL_GCM_IV_GENshipped earlier (tracked in an internal SIM).That guard is only sound against the bypass this PR closes;
together the two commits close the underflow class end-to-end.
clang-formatwas not runlocally; CI will flag drift if any.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.