Fix NULL/OOB read parsing an unmatched bare comp-extra/defaults option on macOS - #141
Open
renard wants to merge 1 commit into
Open
Fix NULL/OOB read parsing an unmatched bare comp-extra/defaults option on macOS#141renard wants to merge 1 commit into
renard wants to merge 1 commit into
Conversation
…n on macOS getsubopt() only guarantees returning -1 for an option that matches none of the given names. It does not guarantee fixing *valuep in that case. glibc sets it to the option's own text, but macOS/BSD libc sets it to NULL. Both compressor_cfg_init_options() (comp_opt.c) and parse_fstree_defaults() (fstree_cli.c) relied on the glibc behavior to look up bare flags such as "extreme" or "hc", or to report an unknown key. On macOS this crashes tar2sqfs/gensquashfs with a NULL strcmp() argument for any --comp-extra flag with no "=value" part (e.g. "extreme", "hc"), and makes --defaults read one element before the start of its option name table on an unrecognized key. Fixed by saving a pointer to the token before each getsubopt() call and using it to recover the option name when getsubopt() returns -1, instead of relying on *valuep. getsubopt() always NUL terminates the consumed token in place before returning, matched or not, so this works the same on every platform. Also dropped parse_fstree_defaults()'s switch "default" case: moving the unrecognized-option check earlier makes it unreachable, and it referenced *valuep the same unsafe way this commit removes everywhere else. Signed-off-by: Sébastien Gross <seb•ɑƬ•chezwam•ɖɵʈ•org>
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
1 participant
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.
getsubopt() only guarantees returning -1 for an option that matches none of the given names. It does not guarantee fixing *valuep in that case. glibc sets it to the option's own text, but macOS/BSD libc sets it to NULL. Both
compressor_cfg_init_options() (comp_opt.c) and
parse_fstree_defaults() (fstree_cli.c) relied on the glibc behavior to look up bare flags such as "extreme" or "hc", or to report an unknown key.
On macOS this crashes tar2sqfs/gensquashfs with a NULL strcmp() argument for any --comp-extra flag with no "=value" part (e.g. "extreme", "hc"), and makes --defaults read one element before the start of its option name table on an unrecognized key.
Fixed by saving a pointer to the token before each getsubopt() call and using it to recover the option name when getsubopt() returns -1, instead of relying on *valuep. getsubopt() always NUL terminates the consumed token in place before returning, matched or not, so this works the same on every platform.
Also dropped parse_fstree_defaults()'s switch "default" case: moving the unrecognized-option check earlier makes it unreachable, and it referenced *valuep the same unsafe way this commit removes everywhere else.