Fuzzing gnustep-base finding #1 This seems legitimate to me. The unit test fails before the fix and passes after.
Full patch available here : https://github.com/poroussel/libs-base/commit/bbbd327b2ab6a5b7747091261eedbb4e4dbf428e Let me know if this is useful, Philippe --- diff --git a/Source/GSString.m b/Source/GSString.m index c102cb73a..b142a9567 100644 --- a/Source/GSString.m +++ b/Source/GSString.m @@ -1194,20 +1194,16 @@ createTinyString(const char *str, int length) return nil; } - /* String too long to fit in a pointer, give up + /* String too long to fit in a pointer, give up. + * Only eight 7-bit characters fit; a ninth character (even a nul) + * would overlap the length and tag bits and produce a string whose + * length is 9 while all the methods assume at most 8 characters. */ - if (length > 9) + if (length > 8) { return nil; } - /* String would fit if the last byte was an implicit 0, but it isn't. - */ - if ((length == 9) && str[8] != '\0') - { - return nil; - } - s |= length << TINY_STRING_LENGTH_SHIFT; for (i = 0 ; i<length ; i++) { diff --git a/Tests/base/NSString/tiny_nine_chars.m b/Tests/base/NSString/tiny_nine_chars.m new file mode 100644 index 000000000..7c748e0fe --- /dev/null +++ b/Tests/base/NSString/tiny_nine_chars.m @@ -0,0 +1,45 @@ +/* + * Nine byte ASCII strings whose last byte is a nul must not be stored as + * tiny strings: only eight 7-bit characters fit in a pointer, and the + * ninth character would overlap the length/tag bits. Found by fuzzing + * GSMimeParser (stack buffer overflow in -[GSTinyString mutableCopyWithZone:]). + */ +#import "Testing.h" +#import <Foundation/NSAutoreleasePool.h> +#import <Foundation/NSString.h> + +int main(int argc, char **argv) +{ + NSAutoreleasePool *arp = [NSAutoreleasePool new]; + const char *bytes = "abcdefgh"; /* 8 chars + nul */ + unichar chars[9] = {'a','b','c','d','e','f','g','h',0}; + NSString *s; + NSString *u; + NSMutableString *m; + + START_SET("nine character string ending in nul") + + s = AUTORELEASE([[NSString alloc] initWithBytes: bytes + length: 9 + encoding: NSASCIIStringEncoding]); + u = AUTORELEASE([[NSString alloc] initWithCharacters: chars length: 9]); + + PASS([s length] == 9, "string has length 9"); + PASS([s characterAtIndex: 8] == 0, "ninth character is a nul"); + PASS([s isEqual: u] && [u isEqual: s], "equals the unichar version"); + PASS([s hash] == [u hash], "hash matches the unichar version"); + + m = AUTORELEASE([s mutableCopy]); + PASS([m length] == 9 && [m characterAtIndex: 8] == 0, + "mutable copy keeps all nine characters"); + PASS_EQUAL(m, u, "mutable copy is equal"); + PASS([s longLongValue] == 0, "longLongValue does not overflow"); + PASS_EQUAL([s substringToIndex: 8], @"abcdefgh", "substring is correct"); + PASS_EQUAL([[s substringToIndex: 8] stringByAppendingString: @"ijklmnop"], + @"abcdefghijklmnop", "appending to the substring works"); + + END_SET("nine character string ending in nul") + + [arp release]; + return 0; +} -- 2.39.5
