New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 5251 link

Starred by 2 users

Issue metadata

Status: Verified
Owner: ----
Closed: Jan 2018
Cc:
Type: Bug-Security



Sign in to add a comment

curl/curl_fuzzer_ftp: Heap-buffer-overflow in loop

Project Member Reported by ClusterFuzz-External, Jan 11 2018

Issue description

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

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

Issue filed automatically.

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

This bug is subject to a 90 day disclosure deadline. If 90 days elapse
without an upstream patch, then the bug report will automatically
become visible to the public.

When you fix this bug, please
  * mention the fix revision(s).
  * state whether the bug was a short-lived regression or an old bug in any stable releases.
  * add any other useful information.
This information can help downstream consumers.

If you have questions for the OSS-Fuzz team, please file an issue at https://github.com/google/oss-fuzz/issues.
 
Project Member

Comment 1 by ClusterFuzz-External, Jan 11 2018

Labels: OS-Linux

Comment 2 by cmeist...@gmail.com, Jan 11 2018

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.

Comment 3 by cmeist...@gmail.com, Jan 11 2018

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.

Comment 5 by cmeist...@gmail.com, Jan 13 2018

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.
0001-ftp-wildcard-fix-matching-an-empty-string-with-a.patch
5.5 KB Download

Comment 7 by cmeist...@gmail.com, Jan 13 2018

Looks pretty reasonable. And the regression test looks fine!
Project Member

Comment 8 by ClusterFuzz-External, Jan 17 2018

Labels: ClusterFuzz-Top-Crash
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

Comment 9 by cmeist...@gmail.com, Jan 17 2018

Daniel fixed this today :)
Project Member

Comment 10 by ClusterFuzz-External, Jan 18 2018

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.
Project Member

Comment 11 by ClusterFuzz-External, Jan 18 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: New)
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
Project Member

Comment 12 by sheriffbot@chromium.org, Feb 18 2018

Labels: -restrict-view-commit
This bug has been fixed for 30 days. It has been opened to the public.

- Your friendly Sheriffbot

Sign in to add a comment