Asterisk
  1. Asterisk
  2. ASTERISK-22590

BufferOverflow in unpacksms16() when receiving 16 bit multipart SMS with app_sms

    Details

    • Frequency of Occurrence:
      Frequent
    • SVN Revision Number:
      399870

      Description

      In the current HEAD, a buffer overflow in app_sms.c prevents Asterisk from receiving 16 bit multipart SMS, as it runs in an endless loop over the array boundaries.

      The function unpacksms16() always expects an even number of bytes to be processed. If, however, the user data header contains an odd number of bytes, the second while-loop never terminates (l is never 0 in the while condition) and it keeps overwriting the boundaries of *i until Asterisk terminates with a SIGSEGFAULT.
      The odd number of bytes are according to specification, though (http://www.etsi.org/deliver/etsi_ts%5C123000_123099%5C123040%5C11.05.00_60%5Cts_123040v110500p.pdf page 74).

      The error has been reproduced by sending a multipart SMS with 16 bit encoding from Deutsche Telekom and Vodafone to a German landline number, which is handled by Asterisk.

      We have addressed this issue by creating a patch, which checks for an odd number of bytes and adds another byte in that case.

      1. Handle16BitSmsWithOddLength.patch
        0.5 kB
        Jan Juergens
      2. smsT8
        1 kB
        Jan Juergens

        Activity

        Hide
        Jan Juergens added a comment -

        The patch that addresses the issue.

        Show
        Jan Juergens added a comment - The patch that addresses the issue.
        Hide
        Matt Jordan added a comment -

        For now, I've locked down this issue to Bug Marshal + Issue Reporter.

        In the future, if you think you've identified something that could be remotely exploitable, please contact a bug marshal or e-mail us at security@asterisk.org. That helps to keep such things from becoming public before they are addressed.

        Show
        Matt Jordan added a comment - For now, I've locked down this issue to Bug Marshal + Issue Reporter. In the future, if you think you've identified something that could be remotely exploitable, please contact a bug marshal or e-mail us at security@asterisk.org. That helps to keep such things from becoming public before they are addressed.
        Hide
        Jan Juergens added a comment -

        We dumped the content of a sms_t struct at the beginning of sms_handleincoming() while trying to understand the issue. I'll add it here so others can reproduce the bug. This data was received from the Deutsche Telekom.

        Show
        Jan Juergens added a comment - We dumped the content of a sms_t struct at the beginning of sms_handleincoming() while trying to understand the issue. I'll add it here so others can reproduce the bug. This data was received from the Deutsche Telekom.
        Hide
        Scott Griepentrog added a comment -

        Jan-

        Can you confirm that in your example smsT8 the text portion of the message is supposed to include the 4 character, as in "...adgj54" is correct instead of "...adgj5"?

        Thanks,
        Scott

        Show
        Scott Griepentrog added a comment - Jan- Can you confirm that in your example smsT8 the text portion of the message is supposed to include the 4 character, as in "...adgj54" is correct instead of "...adgj5"? Thanks, Scott
        Hide
        Jan Juergens added a comment -

        Hi Scott, unfortunately, the phone from which we sent the test messages did not save all the sent messages. I found a message with a similar text passage, so I'm guessing this should be it:

        Ucs2 mit uberlªnge
        1.adgjmptw2.adgjmptw3.adgjmptw4.adgjmptw5.adgj543210.adgjmptw1ªend

        So yes, it looks like the 4 is included.

        Show
        Jan Juergens added a comment - Hi Scott, unfortunately, the phone from which we sent the test messages did not save all the sent messages. I found a message with a similar text passage, so I'm guessing this should be it: Ucs2 mit uberlªnge 1.adgjmptw2.adgjmptw3.adgjmptw4.adgjmptw5.adgj543210.adgjmptw1ªend So yes, it looks like the 4 is included.
        Hide
        Scott Griepentrog added a comment -

        Great! Okay, then this patch duplicates that functionality and should be correct. I'm running it on a test loop trying to break it against random garbage input and it's been fine so far, so there should be no more surprises.

        Show
        Scott Griepentrog added a comment - Great! Okay, then this patch duplicates that functionality and should be correct. I'm running it on a test loop trying to break it against random garbage input and it's been fine so far, so there should be no more surprises.

          People

          • Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development