Skip to content

Fix a theoretical overflow in BIO_printf#2369

Merged
justsmth merged 1 commit intoaws:mainfrom
justsmth:BIO_printf
Apr 29, 2025
Merged

Fix a theoretical overflow in BIO_printf#2369
justsmth merged 1 commit intoaws:mainfrom
justsmth:BIO_printf

Conversation

@justsmth
Copy link
Copy Markdown
Contributor

Found by code inspection. If vsnprintf wanted to write INT_MAX characters, allocating a INT_MAX + 1 scratch buffer will overflow. Since we always have INT_MAX < SIZE_MAX, just casting to size_t earlier avoids this.

(If the malloc implementation is unwilling to allocate INT_MAX + 1, e.g. it is forbidden to on 32-bit, that's malloc's responsibility to detect.)

Change-Id: I3c2a740ebc7ecd58464a9f63858ffcefe67f648f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74247
Auto-Submit: David Benjamin davidben@google.com
Commit-Queue: Adam Langley agl@google.com
Reviewed-by: Adam Langley agl@google.com

Issues:

Resolves #ISSUE-NUMBER1
Addresses #ISSUE-NUMBER2

Description of changes:

Describe AWS-LC’s current behavior and how your code changes that behavior. If there are no issues this pr is resolving, explain why this change is necessary.

Call-outs:

Point out areas that need special attention or support during the review process. Discuss architecture or design changes.

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

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.

Found by code inspection. If vsnprintf wanted to write INT_MAX
characters, allocating a INT_MAX + 1 scratch buffer will overflow. Since
we always have INT_MAX < SIZE_MAX, just casting to size_t earlier avoids
this.

(If the malloc implementation is unwilling to allocate INT_MAX + 1,
e.g. it is forbidden to on 32-bit, that's malloc's responsibility to
detect.)

Change-Id: I3c2a740ebc7ecd58464a9f63858ffcefe67f648f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74247
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
@justsmth justsmth requested review from WillChilds-Klein, samuel40791765 and skmcgrail and removed request for samuel40791765 April 25, 2025 19:45
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.76%. Comparing base (e6afed9) to head (6e24ec6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2369      +/-   ##
==========================================
- Coverage   78.77%   78.76%   -0.02%     
==========================================
  Files         620      620              
  Lines      107958   107958              
  Branches    15329    15329              
==========================================
- Hits        85045    85032      -13     
- Misses      22257    22269      +12     
- Partials      656      657       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@justsmth justsmth changed the title [DRAFT] Fix a theoretical overflow in BIO_printf Fix a theoretical overflow in BIO_printf Apr 28, 2025
@justsmth justsmth marked this pull request as ready for review April 28, 2025 17:22
@justsmth justsmth requested a review from a team as a code owner April 28, 2025 17:22
@justsmth justsmth merged commit 32e73b3 into aws:main Apr 29, 2025
111 of 114 checks passed
@justsmth justsmth deleted the BIO_printf branch April 29, 2025 16:03
@justsmth justsmth mentioned this pull request Apr 30, 2025
justsmth added a commit that referenced this pull request Apr 30, 2025
### Description of changes: 
Bump version to 1.50.1. (Needed so that `aws-lc-sys` can to pick up
latest s2n-bignum changes.)

### Call-outs:
#### What's Changed
* Fix GCC 4.8 docker img; Also w/ GCC 7.5 by @justsmth in
#2344
* Fix LibRdKafka CI by @smittals2 in
#2372
* Expand .clang-tidy configuration by @justsmth in
#2356
* nginx-1.28.0 aws-lc-nginx.patch by @robvanoostenrijk in
#2373
* s2n bignum import method change by @torben-hansen in
#2324
* Fix a theoretical overflow in BIO_printf by @justsmth in
#2369
* Fix tpm2-tss integration test by @justsmth in
#2370


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.
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.

5 participants