Issue 5251: curl/curl_fuzzer_ftp: Heap-buffer-overflow in loop

Hooray, more issues with the fnmatch parser! :)

)$ python read_corpus.py --input ../clusterfuzz-testcase-minimized-5917474819145728
TLVHeader(type='CURLOPT_URL' (1), length=37, data='ftp:/%f26.0X0.1:8991/[-list/*[!*-]*-]')
TLVHeader(type='Server banner (sent on connection)' (2), length=11, data='220 Hello!\n')
TLVHeader(type='Server response 1' (17), length=9, data='200 Sure\n')
TLVHeader(type='Server response 2' (18), length=9, data='200 Sure\n')
TLVHeader(type='Server response 3' (19), length=9, data='200 Su2e\n')
TLVHeader(type='Server response 4' (20), length=9, data='400 Sure\n')
TLVHeader(type='Server response 5' (21), length=50, data='227 E\x88\x88\x88\x88\x88\x88\x88\x88Pass bytesd\xff=(2$3,229,112,127,226,4)\n')
TLVHeader(type='Server response 6' (22), length=9, data='210 Sure\n')
TLVHeader(type='Server response 7' (23), length=34, data="150 'ucc\rssS\n226 Ever\x00\x00\x00\t201 rSu\ne")
TLVHeader(type='Socket 2: Server banner (sent on connection)' (31), length=570, data='total 20\r\nDrwxr-xr-x   8 98   @charset=UTF-7   2 compresc 9 bin ->n -    0>   .06 .Ne:T\xfd\xf5srwxrwxrwx   1   0 )    7 Dec  3  1999 bio  .06 .NI:T-\nsrwxrwxrwx   1   0 )    7 9ec  8  1999  .Ne:T\r\nbrwxrwxrwx   1   \xd8 )    7 Dec  8  1999 bin  .06->n   0 \x12.06 .\nbrwxrwxrwx   1   8 )    7 Dec  8  1999 \x98in  EADM=\r\n-r--r--r--   0 0 ;t\xbcpe= 1         s 9 bin 0 ) \x00 =UTF-6   26 .Ne:T\r\nbrwxrwxrwx   1   1 )    7 Dec  8  1999 bin  .06->n   0 \x12.06 .\nbrwxrwxrwx   1   8 )    7 Dec  8  1999 \xe2in  EADM\xb7\xf2\xf5\xd2\x8d\xd2\xd2\x8d--r--   0 0 ;t\xbcpe= 1             H5  H5 Jul 16\x00  ->n -    0>q999 bin)\x01> 0     r\r\n')
TLVHeader(type='CURLOPT_WILDCARDMATCH' (33), length=4, data='\x00\xc4\x00\x03')
2018-01-11 20:50:39,216 INFO  Returning 0


Stepping through, it looks like the function is running off the end of the buffer.

Breakpoint 3, loop (pattern=0x60300002bf68 "*[!*-]*-]",
    string=0x60f00000061e "->n -    0>   .06 .Ne:T\375\365srwxrwxrwx   1   0 )    7 Dec  3  1999 bio  .06 .NI:T-") at curl_fnmatch.c:318
318             if(*s == '\0' && *(p + 1) == '\0')
2: p = (unsigned char *) 0x60300002bf68 "*[!*-]*-]"
3: s = (unsigned char *) 0x60f00000066c "-"
(gdb)
Continuing.

Breakpoint 3, loop (pattern=0x60300002bf68 "*[!*-]*-]",
    string=0x60f00000061e "->n -    0>   .06 .Ne:T\375\365srwxrwxrwx   1   0 )    7 Dec  3  1999 bio  .06 .NI:T-") at curl_fnmatch.c:318
318             if(*s == '\0' && *(p + 1) == '\0')
2: p = (unsigned char *) 0x60300002bf68 "*[!*-]*-]"
3: s = (unsigned char *) 0x60f00000066d ""
(gdb)
Continuing.

Breakpoint 3, loop (pattern=0x60300002bf69 "[!*-]*-]", string=0x60f00000066d "")
    at curl_fnmatch.c:318
318             if(*s == '\0' && *(p + 1) == '\0')
2: p = (unsigned char *) 0x60300002bf6e "*-]"
3: s = (unsigned char *) 0x60f00000066e '\276' <repeats 34 times> <<<<<<<<<<<<<<<<<<<<<<<<< We carry on until we exhaust the end of the buffer
(gdb)
Continuing.

This trace hopefully shows the issue:

309       unsigned char charset[CURLFNM_CHSET_SIZE] = { 0 };
2: p = (unsigned char *) 0x60300002bf69 "[!*-]*-]"
3: s = (unsigned char *) 0x60f00000066d ""
(gdb)
310       int rc = 0;
313         switch(state) {
315           if(*p == '*') {
328           else if(*p == '?') {
338           else if(*p == '\0') {
343           else if(*p == '\\') {
347           else if(*p == '[') {
348             unsigned char *pp = p + 1; /* cannot handle with pointer to register */
349             if(setcharset(&pp, charset)) {
350               int found = FALSE;
351               if(charset[(unsigned int)*s])
353               else if(charset[CURLFNM_ALNUM])
355               else if(charset[CURLFNM_ALPHA])
357               else if(charset[CURLFNM_DIGIT])
359               else if(charset[CURLFNM_XDIGIT])
361               else if(charset[CURLFNM_PRINT])
363               else if(charset[CURLFNM_SPACE])
365               else if(charset[CURLFNM_UPPER])
367               else if(charset[CURLFNM_LOWER])
369               else if(charset[CURLFNM_BLANK])
371               else if(charset[CURLFNM_GRAPH])
374               if(charset[CURLFNM_NEGATE])
375                 found = !found;
377               if(found) {
378                 p = pp + 1;
379                 s++;   <<<<< nope! incrementing past the end of a string
380                 memset(charset, 0, CURLFNM_CHSET_SIZE);
2: p = (unsigned char *) 0x60300002bf6e "*-]"
3: s = (unsigned char *) 0x60f00000066e '\276' <repeats 34 times>
(gdb)

This function feels a bit weird in general. Maybe an up-front check for empty strings might help?

I don't think an up-front check for a zero length string won't really work since the asterisk "*" is meant to also match no characters.

"*[^a]" would return true for an empty string, but "*[a]" will not.

Ah yeah, good point.

Ok then, I'll defer to your better judgement on this one!

Here's my proposed patch, with this test case added to test 1307 for verification.

Deleted: 0001-ftp-wildcard-fix-matching-an-empty-string-with-a.patch 5.5 KB

0001-ftp-wildcard-fix-matching-an-empty-string-with-a.patch 5.5 KB Download

Looks pretty reasonable. And the regression test looks fine!

Testcase 5917474819145728 is a top crash on ClusterFuzz for linux platform. Please prioritize fixing this crash.

If this is incorrect, please file a bug on https://github.com/google/oss-fuzz/issues/new

Daniel fixed this today :)

ClusterFuzz has detected this issue as fixed in range 201801170522:201801180524.

Detailed report: https://oss-fuzz.com/testcase?key=5917474819145728

Project: curl
Fuzzer: libFuzzer_curl_fuzzer_ftp
Fuzz target binary: curl_fuzzer_ftp
Job Type: libfuzzer_asan_curl
Platform Id: linux

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x60f000000780
Crash State:
  loop
  loop
  ftp_pl_insert_finfo
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://oss-fuzz.com/revisions?job=libfuzzer_asan_curl&range=201710290615:201710300415
Fixed: https://oss-fuzz.com/revisions?job=libfuzzer_asan_curl&range=201801170522:201801180524

Reproducer Testcase: https://oss-fuzz.com/download?testcase_id=5917474819145728

See https://github.com/google/oss-fuzz/blob/master/docs/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 5917474819145728 is verified as fixed, so closing issue as verified.

If this is incorrect, please file a bug on https://github.com/google/oss-fuzz/issues/new

This bug has been fixed for 30 days. It has been opened to the public.

- Your friendly Sheriffbot