You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
Hi! We've been fuzzing unbound with sydr-fuzz security predicates and we found numeric truncation error in str2wire.c:1978 and str2wire.c:2023.
In function sldns_str2wire_nsec_buf on line 2022 variable t has type uint16_t, function sldns_get_rr_type_by_name returns enum type sldns_rr_type that have unsigned int type value. The same is in sldns_str2wire_type_buf on line 2067. This enum has values that >= 65535. Out tool has found input where this function returns atoi(name + 4) and its value is bigger than 65535, so the numeric truncation occurs. So we suggest to add an assert checker in if operator in function sldns_get_rr_type_by_name on sldns/rrdef.c:704:
if (strlen(name) > 4 && strncasecmp(name, "TYPE", 4) == 0) {
unsigned int a = atoi(name + 4);
assert(a <= LDNS_RR_TYPE_LAST);
return a;
}
sldns/str2wire.c:2022:16: runtime error: implicit conversion from type 'sldns_rr_type' (aka 'enum sldns_enum_rr_type') of value 4294189519 (32-bit, unsigned) to type 'uint16_t' (aka 'unsigned short') changed the value to 8655 (16-bit, unsigned)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior sldns/str2wire.c:2022:16
sldns/str2wire.c:2067:15: runtime error: implicit conversion from type 'sldns_rr_type' (aka 'enum sldns_enum_rr_type') of value 4294189519 (32-bit, unsigned) to type 'uint16_t' (aka 'unsigned short') changed the value to 8655 (16-bit, unsigned)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior sldns/str2wire.c:2067:15
The assert() may or may not run depending on whether the program is compiled with NDEBUG defined or not, but more importantly Unbound should not crash with an assertion in response to user input.
A better fix would be to make this function return an explicit failure code if the input isn't valid and update the callers of it to handle invalid input.
The assert() may or may not run depending on whether the program is compiled with NDEBUG defined or not, but more importantly Unbound should not crash with an assertion in response to user input.
A better fix would be to make this function return an explicit failure code if the input isn't valid and update the callers of it to handle invalid input.
@edmonds
I have added new enum LDNS_RR_TYPE_ERROR = LDNS_RR_TYPE_LAST - 1 in rrdefs.h and parsed case of this enum in functions where sldns_get_rr_type_by_name is called (except testcode/dohclient.c).
Hi headshog, thanks for putting in the effort! By looking at the existing code I would return (enum sldns_enum_rr_type)0. Calling functions already check for something like (*tp == 0 && strcmp(token, "TYPE0") != 0) as an error condition. Also the same exact issue is present in the sldns_get_rr_class_by_name() function for the CLASSxx representation.
Hi headshog, thanks for putting in the effort! By looking at the existing code I would return (enum sldns_enum_rr_type)0. Calling functions already check for something like (*tp == 0 && strcmp(token, "TYPE0") != 0) as an error condition. Also the same exact issue is present in the sldns_get_rr_class_by_name() function for the CLASSxx representation.
@gthess Hi! I've applied changes that you requested.
Looks good apart from the return type used for sldns_get_rr_class_by_name().
It would be great if you also implement the (*tp == 0 && strcmp(token, "TYPE0") != 0) type of check when these are called, like you did in one of your previous commits :)
Looks good apart from the return type used for sldns_get_rr_class_by_name(). It would be great if you also implement the (*tp == 0 && strcmp(token, "TYPE0") != 0) type of check when these are called, like you did in one of your previous commits :)
gthess
changed the title
Numeric truncation at str2wire.c at lines 2022 and 2067
Numeric truncation when parsing TYPEXX and CLASSXX representation
Jul 20, 2023
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.
gthess
changed the title
Hi! We've been fuzzing
unboundwith sydr-fuzz security predicates and we found numeric truncation error instr2wire.c:1978andstr2wire.c:2023.In function
sldns_str2wire_nsec_bufon line 2022 variablethas typeuint16_t, functionsldns_get_rr_type_by_namereturns enum typesldns_rr_typethat haveunsigned inttype value. The same is insldns_str2wire_type_bufon line 2067. This enum has values that >= 65535. Out tool has found input where this function returnsatoi(name + 4)and its value is bigger than 65535, so the numeric truncation occurs. So we suggest to add an assert checker inif operatorin functionsldns_get_rr_type_by_nameonsldns/rrdef.c:704:Environment
How to reproduce this error
Build docker container:
Run docker container:
Run on the following input:
Output: