Fenrir fixes#10803
Open
Frauschi wants to merge 8 commits into
Open
Conversation
The Camellia encrypt and decrypt operations used the key schedule without checking that a key had ever been configured. A zeroed or otherwise unkeyed context has a keySz that does not match 128, 192, or 256, so the underlying block transform hit the default no-op case and CBC emitted an easily reversible XOR chain while still returning success. A caller who forgot wc_CamelliaSetKey received a success code with effectively unencrypted output. Add a key-state check that accepts only valid Camellia key sizes and have wc_CamelliaEncryptDirect, wc_CamelliaDecryptDirect, wc_CamelliaCbcEncrypt, and wc_CamelliaCbcDecrypt return MISSING_KEY when no key has been set. Mirrors the existing 3DES keySet guard. Add a regression test covering the unkeyed and garbage key-size paths.
The RC2 encrypt and decrypt operations used the expanded key schedule without checking that a key had ever been configured. On a zeroed or otherwise unkeyed context the ECB ops ran over an all-zero schedule and returned success, and the CBC wrappers inherited the same behavior, so a caller who skipped wc_Rc2SetKey received ciphertext under an unintended key with no error signalled. Guard wc_Rc2EcbEncrypt and wc_Rc2EcbDecrypt on a zero keylen and return MISSING_KEY when no key has been set. The CBC wrappers call these and propagate the error. Mirrors the existing 3DES keySet guard. Add a regression test covering the unkeyed path for all four ops.
wolfSSL_BIO_write rejected negative lengths but allowed a large positive length through to wolfSSL_BIO_MEMORY_write. On a fresh buffer an INT_MAX length overflowed the 4/3 buffer growth calculation, so the grow reported success with a short allocation and the following copy read far past the small source buffer. Add an upper bound check that rejects lengths large enough to overflow the growth math before any allocation or copy, and add a regression test that drives a huge length through the public BIO_write entry point.
wolfSSL_EVP_EncodeBlock rejected negative input lengths but passed any large positive length straight to Base64_Encode_NoNl, which read that many bytes from the caller input buffer and ran past its allocation. Reject input lengths whose base64 output would overflow a positive int, which also bounds the read against the caller allocation. The encoded length is the int return value, so the safe maximum input is (INT_MAX / 4) * 3.
wolfSSL_EVP_EncodeUpdate did not validate the input length. A large inl caused the block loop and the residual copy to read far past the caller's input buffer, and a negative inl was silently treated as success. Reject negative lengths and lengths whose base64 output would overflow a positive int before processing any data.
An oversized length argument was passed straight to GetASNHeader as the buffer bound. A caller supplying a length larger than the real buffer let the OBJECT_ID header claim more content than was present, driving the OID validation read past the end of the allocation. Since an ASN1_OBJECT is an OID, clamp the parse window to the maximum OID encoding so the header decode cannot read beyond a sane bound.
When the caller passes the object's own data pointer as the source, wolfSSL_ASN1_STRING_set freed the existing buffer before copying from it, reading freed memory in the dynamic case and copying cleared bytes in the fixed-buffer case. Duplicate the source into a temporary buffer when it aliases the object before disposing of the old buffer, then free the temporary once the copy completes.
QUIC performs key updates at the packet-protection layer via the Key Phase bit, so RFC 9001 section 6 requires a QUIC endpoint to reject any received TLS KeyUpdate handshake message as a fatal unexpected_message connection error and to never send one. The TLS 1.3 receive path processed the message normally, rotating traffic secrets and possibly emitting a prohibited KeyUpdate response, and the send path allowed a QUIC connection to originate a KeyUpdate. Guard the key_update case in SanityCheckTls13MsgReceived so a QUIC connection aborts with a fatal unexpected_message alert, and guard Tls13UpdateKeys so a QUIC connection cannot send a KeyUpdate. Add a QUIC unit test that feeds a post-handshake KeyUpdate and confirms the connection is refused.
|
retest this please |
|
Contributor
Author
|
Jenkins retest this please |
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
2 participants
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.
Fix various Fenrir issues:
EVP_EncodeBlockto prevent the base64 output-size calculation from overflowing.EVP_EncodeUpdateto stop integer overflow and out-of-bounds input reads.d2i_ASN1_OBJECTparse window to the encoded OID size to avoid reading beyond the object.wolfSSL_ASN1_STRING_setwhen the string is set from an alias of its own buffer.MISSING_KEYinstead of running on uninitialized state.