fix: sign more headers safely, and order prefix names correctly - #9
Merged
Conversation
AWS rejects a request when any x-amz-* header it carries is missing from SignedHeaders, so callers need to pass all of them through. Three things got in the way: - the header limit was 10, and the internal arrays were sized to that same limit while host, x-amz-date and x-amz-content-sha256 are appended on top of the caller's headers -- 8 caller headers already wrote past both stack arrays. Limit is now 24, arrays are sized for the appended ones. - the comparators used strncmp over the shorter name, so names that share a prefix (x-amz-server-side-encryption and its -aws-kms-key-id variant on SSE-KMS requests) compared equal and could be emitted in either order, producing a canonical form AWS does not accept. Ties now break on length. - canonical request, string to sign and the Authorization value were written with unbounded sprintf into fixed stack buffers, checked for overflow only after the fact, and aws_sigv4_sign discarded the overflow return entirely. All writes are now clamped and the error is propagated. The canonical request buffer goes 2048 -> 4096, since a session token alone can take 1-2 KB, and string to sign 2048 -> 1024, which is far more than the fixed ~140 bytes it holds. Tests cover an x-amz-copy-source (CopyObject) signature, prefix-name ordering and the overflow path; the existing vector is unchanged.
pbalogh-sa
approved these changes
Aug 3, 2026
baluchicken
approved these changes
Aug 3, 2026
stoader
approved these changes
Aug 3, 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.
AWS rejects a request when any
x-amz-*header it carries is missing fromSignedHeaders("There were headers present in the request which were not signed"), so callers have to pass all of them in. Doing that hit three problems here:AWS_SIGV4_MAX_NUM_HEADERSwas 10 and the local arrays inget_signed_headers/get_canonical_headerswere sized to it, whilehost,x-amz-dateandx-amz-content-sha256are appended on top of the caller's headers — 8 caller headers already wrote past both arrays. Limit is now 24 and the arrays are sized for the appended entries.strncmpover the shorter name, so names sharing a prefix (x-amz-server-side-encryptionandx-amz-server-side-encryption-aws-kms-key-idon SSE-KMS requests) compared equal and could be emitted in either order — a canonical form AWS won't accept. Ties now break on length, shorter first.Authorizationvalue were written withaws_sigv4_sprintf(no limit) into fixed buffers and only length-checked afterwards, andaws_sigv4_signdroppedget_canonical_request's overflow return. All writes are clamped to the buffer end now and the error is propagated, so a too-large request fails instead of corrupting the caller's stack.Buffers: canonical request 2048 → 4096 (a session token alone can be 1–2 KB), string-to-sign 2048 → 1024 (it holds a fixed ~140 bytes).
get_signed_headers,get_canonical_headers,get_canonical_request,get_credential_scopeandget_string_to_signtake alastlimit; the latter now returns a status.aws_sigv4_signis unchanged for callers.Testing
make test— 4/4 pass. New cases: anx-amz-copy-source(CopyObject) signature, SSE-KMS prefix-name ordering, and the overflow path returningAWS_SIGV4_BUFFER_OVERFLOW_ERROR. The existing test vector is byte-identical. All signatures were cross-checked against an independent SigV4 implementation written from the AWS spec, and the suite was also run under ASan/UBSan with no reports.