Fix buffer overflows (CVE-2021-45958) #519
Conversation
Add a few extra memory reserve calls to account for the extra space that indentation needs. These kinds of memory issues are hard to spot because the buffer is resized in powers of 2 meaning that a miscalculation would only show any symptoms if the required buffer size is estimated to be just below a 2 power but is actually just above. Add a debug mode which replaces the 2 power scheme with reserving only the memory explicitly requested and adds some overflow checks.
Unsetting it can lead to seg-faults. I don't think it's worth having to fix and then test this undocumented permutation.
* Removed the reservations in Buffer_EscapeStringUnvalidated and Buffer_EscapeStringValidated as those are not needed and may hide other bugs. * Debug check in Buffer_EscapeStringValidated was triggering incorrectly. * The reservation on JT_RAW was much larger than necessary; the value is copied directly, so the factor six is not needed, and this may hide other bugs. * Explicit accurate reservations everywhere else.
If the default output format changes in the future (e.g. `separators` as in the standard library), these tests would otherwise become irrelevant.
Codecov Report
@@ Coverage Diff @@
## main #519 +/- ##
==========================================
+ Coverage 90.41% 90.63% +0.22%
==========================================
Files 6 6
Lines 1752 1783 +31
==========================================
+ Hits 1584 1616 +32
+ Misses 168 167 -1
Continue to review full report at Codecov.
|
|
|
||
| - name: Install | ||
| run: | | ||
| python -m pip install -U pip |
There was a problem hiding this comment.
Interesting there's a Python available without actions/setup-python, looks like it has Python 3.8.
In any case, shall we include an explicit actions/setup-python so we don't get any surprises when the version changes? Can we use 3.10?
There was a problem hiding this comment.
Ubuntu ships a Python with it - that's the one that's there by default. It'll get upgraded whenever a new ubuntu image is added to GitHub Actions and ubuntu-latest shifts. It's a complete Python3+pip environment and this test shouldn't be Python version dependent so I'd be happy to use it as is.
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
|
Nice one @JustAnotherArchivist. @hugovk How long (if at all) do we need to twiddle our thumbs before pushing the release? |
|
Release early, release often! We can do one right now if you're ready, it's just three clicks away! (https://github.com/ultrajson/ultrajson/blob/main/RELEASING.md) Would you like to do the honours? |
|
Button is pressed. That's scarily streamlined. |
|
This is now released as 5.2.0: Thank you very much @bwoodsend and @JustAnotherArchivist for the PR and also to @gsnedders for your help with reviews! |
Port fixes and unit tests from ultrajson/ultrajson#519 to fix buffer overflows (CVE-2021-45958)
Port fixes and unit tests from ultrajson/ultrajson#519 to fix buffer overflows (CVE-2021-45958)
Port ultrajson/ultrajson#519 (Fix buffer overflows)
Fixes #334
Fixes #501
Fixes #502
Fixes #503
This is a replacement of #504 and should properly fix the buffer overflows. In addition to @bwoodsend's changes in that PR, this is a rewrite of the buffer reservation calls from scratch and fixes a bug in the debug buffer check (
RESERVE_STRINGincludes the double quotes around a string, so the check for adding one character would fail in certain conditions when addition was actually safe because it would check for 8 bytes instead of 6). I also widened the test cases from #501 and #503 so they wouldn't become obsolete on possible future default output format changes. Rebased onto main due to the black/Click incompatibility failing tests as well.