Fix float-to-int conversion UB in parser and rasterizer (#292, #293, #294) - #298
Open
1820893135-pixel wants to merge 1 commit into
Open
1820893135-pixel wants to merge 1 commit into
1820893135-pixel wants to merge 1 commit into
Conversation
Three related undefined-behavior bugs where a float value that has overflowed to inf/NaN (or whose fixed-point product exceeds INT_MAX) is cast to int: - nsvg__addActive(): nsvg__roundf(NSVG__FIX * ...) can exceed INT_MAX for extreme coordinates, so casting it to int is UB and can produce a negative delta/x that corrupts the active-edge list. Clamp via nsvg__roundf_clamp() and saturate the x stepping with nsvg__iadd_sat(). - nsvg__curveDivs(): for a huge stroke width, r/(r+tol) rounds to 1.0f, acosf() returns 0, and arc/da becomes +inf; casting that to int is UB. Fall back to the minimum subdivision count. - nsvg__pathArcTo(): a degenerate arc can make the delta angle NaN or +/-inf, making (int)(fabsf(da) / (NSVG_PI*0.5f) + 1.0f) UB. Degenerate to a straight line in that case. Fixes memononen#292, memononen#293 and memononen#294.
jantzeno
added a commit
to jantzeno/nanosvg_slop
that referenced
this pull request
Sep 21, 2026
Summary: - Adapt the original numeric patch with explicit NaN/infinity handling, portable integer limits, and overflow-safe scanline addition. - Guard stroke subdivision counts, downstream round-join casts, and arc conversion; degenerate invalid arcs to lines when endpoints are finite. - Cover numeric boundaries, huge coordinates and stroke widths, non-finite radii/angles, the non-dashed large-segment control, and ordinary pixels. Tests: - `CC=/usr/bin/clang sh tests/run.sh` — passed address, undefined-behavior, and float-cast-overflow sanitizer checks. - `CC=/usr/bin/clang++ CFLAGS='-x c++ -std=c++11 -O1 -g -fsanitize=address,undefined,float-cast-overflow -fno-sanitize-recover=all' sh tests/run.sh` — passed. - A sanitizer probe built against the previous commit reproduced numeric failures for edge-overflow, stroke-overflow, arc-overflow, and no-dash SVGs. - `git diff --check` — passed. Original patch and reports by @1820893135-pixel: https://github.com/1820893135-pixel Based-on: memononen#298 Source: memononen#292 Source: memononen#293 Source: memononen#294
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.
These three issues are the same class of bug: a float value that has overflowed to
inf/NaN(or whose fixed-point product exceedsINT_MAX) is cast toint, which is undefined behavior (CWE-681/CWE-190). They are all in the parser/rasterizer's float→fixed-point path, so I bundled them into one PR.#292 —
nsvg__addActive: fixed-point coordinate overflownsvg__roundf(NSVG__FIX * ...)can exceedINT_MAXfor extreme coordinates, so casting it tointis UB and can yield a negative delta/x that corrupts the active-edge list.nsvg__roundf_clamp()(clamps before casting) andnsvg__iadd_sat()(saturating add).z->dxandz->xnow use the clamped helpers; the scanline stepping usesnsvg__iadd_sat(z->x, z->dx).#293 —
nsvg__curveDivs: stroke-tessellation count overflowFor a huge stroke width,
r/(r+tol)rounds to1.0f, soacosf()returns0andarc/dabecomes+inf; casting that tointis UB.if (!(da > 0.0f)) return 2;to fall back to the minimum subdivision count.#294 —
nsvg__pathArcTo: NaN-to-int in arc conversionA malformed arc can make the delta angle
daNaN or±inf, so(int)(fabsf(da) / (NSVG_PI*0.5f) + 1.0f)is UB.dais non-finite, degenerate to a straight line (same handling as the existing degeneracy checks).All three fixes were verified with the fuzzer harness under UBSan (each previously crashed with
float-cast-overflow; after the fix they exit cleanly), and a 200-seed regression run showed no behavioral differences versus the unfixed build.Fixes #292, #293, #294.