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

Issue metadata

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



Sign in to add a comment

curl/curl_fuzzer_pop3: Heap-buffer-overflow in pop3_get_message

Project Member Reported by ClusterFuzz-External, Jan 10

Issue description

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

Project: curl
Fuzzer: afl_curl_fuzzer_pop3
Fuzz target binary: curl_fuzzer_pop3
Job Type: afl_asan_curl
Platform Id: linux

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x629000013852
Crash State:
  pop3_get_message
  Curl_sasl_continue
  pop3_state_auth_resp
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://oss-fuzz.com/revisions?job=afl_asan_curl&range=201712050515:201712060515

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

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 10

Labels: OS-Linux
Looks very similar to the bug fixed in https://github.com/curl/curl/pull/2150.

Server response appears to be ~17KB of stuff - obviously something's not working quite right there. I can look into this later when I have a solid internet connection.
Ahh. Ok. Bug is obvious from code read; probably should have caught this at review stage...

@bagder: not sure when you're back off holiday; do you want to handle this one?

Current code is as follows:

    static void pop3_get_message(char *buffer, char **outptr)
    {
      size_t len = strlen(buffer);
      char *message = NULL;

      if(len > 2) {
        /* Find the start of the message */
        for(message = buffer + 2; *message == ' ' || *message == '\t'; message++)
          ;

        /* Find the end of the message */
        for(len -= 2; len--;)
          if(message[len] != '\r' && message[len] != '\n' && message[len] != ' ' &&
             message[len] != '\t')
            break;

        /* Terminate the message */
        if(++len) {
          message[len] = '\0';
        }
      }
      else
        /* junk input => zero length output */
        message = &buffer[len];

      *outptr = message;
    }

We get the length of the buffer and put it in `len`. We then increment `message` to try and find the start of the message. However - we don't reduce `len` by the same amount!

POC:
buffer is '\x01\x01\t\t\x00'
strlen(buffer) = 4
len > 2, so we try and find the start of the message
message ends up equalling &buffer[4]
len -= 2 (= 2)
and we're trying to access message[2], which is buffer[6], and thus we're off the end of the buffer.

Fix:
decrement the len counter sensibly! Something like this?

      if(len > 2) {
        /* Find the start of the message */
        for(message = buffer + 2; *message == ' ' || *message == '\t'; message++)
          ;

        // Work out how much length is left in the buffer. Do this by reducing
        // len by the message offset from the start of the buffer.
        len -= (message - buffer);

        /* Find the end of the message */
        for(; len--;)
          if(message[len] != '\r' && message[len] != '\n' && message[len] != ' ' &&
             message[len] != '\t')
            break;
I propose we decrease len in the lopp where message is increased. I find that more readable/understandable.

The same problem exists in all pop3/smtp/imap versions of that function. My proposed patch is attached.
0001-get_message-decrease-the-data-length-too.patch
4.4 KB Download
Patch looks fine to me!
Merged in commit 8dd4edeb9
Project Member

Comment 8 by ClusterFuzz-External, Jan 16

ClusterFuzz has detected this issue as fixed in range 201801150545:201801160525.

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

Project: curl
Fuzzer: afl_curl_fuzzer_pop3
Fuzz target binary: curl_fuzzer_pop3
Job Type: afl_asan_curl
Platform Id: linux

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x629000013852
Crash State:
  pop3_get_message
  Curl_sasl_continue
  pop3_state_auth_resp
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://oss-fuzz.com/revisions?job=afl_asan_curl&range=201712050515:201712060515
Fixed: https://oss-fuzz.com/revisions?job=afl_asan_curl&range=201801150545:201801160525

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

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 9 by ClusterFuzz-External, Jan 16

Labels: ClusterFuzz-Verified
Status: Verified (was: New)
ClusterFuzz testcase 5702643004473344 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 10 by sheriffbot@chromium.org, Feb 15

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