ls-config: bound the sinp scratch buffer in sscanf() calls - #299
Merged
hyperrealm merged 1 commit intoSep 28, 2026
Merged
Conversation
contrib/ls-config copies each command line argument value (-s/-g/-d/-p/-f
and their long-option equivalents) into a 256-byte heap buffer called
sinp before duplicating it into its own allocation. Every one of those
copies went through sscanf(optarg, "%s", sinp) or
sscanf(optarg, "%[^\n]s", sinp), neither of which carries a field width,
so an argument of a few hundred bytes writes straight past the end of
the buffer.
Built with -fsanitize=address and run as:
ls-config -s "$(python3 -c 'print("A"*500)')" -f /dev/null
this aborts with a heap-buffer-overflow write of 501 bytes into the
256-byte sinp allocation, right at the sscanf() call on the -s path.
The fix gives every one of the ten call sites a field width of 255
(leaving room for the terminating NUL), pulled from one SINP_MAXLEN
definition next to the buffer's size so they can't drift apart again.
Added contrib/ls-config/src/test_sinp_overflow.c, which pulls in the
real ls-config.c (with main renamed out of the way) and drives it with
an oversized --set argument. Checked that it aborts under ASan against
the unpatched file and completes cleanly against the fix.
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.
Fixes #289.
contrib/ls-configcopies each command line argument value (-s/-g/-d/-p/-f, and the matching long options) into a 256-byte heap buffer calledsinpbefore duplicating it into its own allocation. All ten of those copies go throughsscanf(optarg, "%s", sinp)orsscanf(optarg, "%[^\n]s", sinp), and neither conversion carries a field width, so any argument longer than the buffer writes straight past the end of it.Built with
-fsanitize=addressand run as:this aborts immediately with a heap-buffer-overflow write of 501 bytes into the 256-byte
sinpallocation, right at thesscanf()call on the-spath (ls-config.c:1249in the version I started from). Every other option that reads intosinphas the identical problem, since they all share the same buffer and the same unbounded format strings.The fix adds a field width of 255 (leaving room for the NUL) to all ten
sscanf()calls, driven from oneSINP_MAXLENdefinition placed next to where the buffer is sized, so the two can't silently drift apart again in a future edit.I added
contrib/ls-config/src/test_sinp_overflow.c, which pulls in the actualls-config.c(withmainrenamed out of the way via a macro) and drives it with an oversized--setargument, the same way the shell repro above does. Under ASan it aborts against the unpatched file and completes cleanly against the fix; I verified both directions by building it against a stashed copy of the original code before restoring the patch.contrib/ls-configdoesn't currently have any test wiring of its own (it's a standalone tool built via its own makefile, separate from the library's CMake/tinytest setup), so this is a standalone regression test rather than something hooked intomake check— happy to adjust if there's a preferred place for it.Tested on macOS with clang, linking against a locally-built
libconfig.aand Homebrew'sgettextforlibintl.h(not available on macOS by default).