Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix seg fault(s) in AuthV4Signer, test more combos with checksumming #2806

Merged

Conversation

jeking3
Copy link
Contributor

@jeking3 jeking3 commented Jan 5, 2024

Issue #, if available:

V1183021468

Description of changes:

I observed a null pointer dereference in the following conditions:

  1. Calling PutObject with no body (zero length object).
  2. Setting the checksum algorithm to SHA256 (though likely any will do).
  3. Providing a pre-calculated checksum (though likely any matching (2) will do).
  4. Using payload signatures.

In multiple places, SDK code checks to see if the Content-Body is a nullptr before using it, but in AWSAuthV4Signer::SignRequest it was not doing so.

A second bug was observed when not using payload signatures, only when the scheme is HTTP. In this case the code assumed it was okay to call GetHeaderValue on Content-Length before checking if the header exists (it is not okay to do so). This was identified by the more rigorous testing.

Added a matrix test to verify more code paths in AWSAuthV4Signer and fixed the two seg faults in the area. The assumptions checked during testing should be scrutinized.

Tested this live against s3 and s3express endpoints using a workload that generates zero-length objects.

Check all that applies:

  • Did a review by yourself: Yes
  • Added proper tests to cover this PR. (If tests are not applicable, explain.): Yes
  • Checked if this PR is a breaking (APIs have been changed) change: No
  • Checked if this PR will not introduce cross-platform inconsistent behavior: No
  • Checked if this PR would require a ReadMe/Wiki update: No

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

When issuing PutObject with zero length, and setting the checksum
algorithm to SHA256, and setting the checksum value (instead of letting
the SDK calculate it) resulted in a null pointer dereference with
payload signing enabled.

Added a matrix test to verify more code paths in AWSAuthV4Signer and
fixed two seg faults.
@SergeyRyabinin SergeyRyabinin merged commit 64b91a5 into aws:main Jan 5, 2024
2 of 3 checks passed
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.

2 participants