[asterisk-bugs] [JIRA] (ASTERISK-25857) func_aes: incorrect use of strlen() leads to data corruption

[Gianluca Merlo (JIRA)]

Gianluca Merlo created ASTERISK-25857:
-----------------------------------------

             Summary: func_aes: incorrect use of strlen() leads to data corruption
                 Key: ASTERISK-25857
                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-25857
             Project: Asterisk
          Issue Type: Bug
      Security Level: None
          Components: Functions/func_aes
    Affects Versions: 13.7.2, 13.6.0, 11.21.2
            Reporter: Gianluca Merlo


Hello,

I tried to use {{AES_ENCRYPT}}/{{AES_DECRYPT}} from {{func_aes}} for reasons similar to the ones in issue leading to their addition (ASTERISK-13422).

In my case, the integrity of data passing through was fortunately monitored by validating their format. On all versions I tried, I observed that some of the data seemed "corrupted". I took interest in the issue, and ran encryption/decription stress tests by placing a call to a dialplan snippet such as:

{noformat}
exten => _[a-zA-Z0-9].,1,Noop
    same => n,Set(KEY=abcdefghijklmnop)
    same => n,Ringing()
    same => n,Wait(5)
    same => n,Answer()
    same => n,Set(TIMEOUT(absolute)=120)
    same => n,Set(cycles=0)
    same => n,While($[ ${INC(cycles)} < 10000 ])
        same => n,Set(original=${RAND()}:${RAND()}:${RAND()}:${RAND()})
        same => n,Set(crypt=${AES_ENCRYPT(${KEY},${original})})
        same => n,GotoIf($[ ${LEN(${crypt})} > 0 ]?decrypt)
        same => n,Log(NOTICE,TEST is NULL - orig <${original}> crypt <${crypt}> decrypt <>)
        same => n,ContinueWhile()
        same => n(decrypt),Set(decrypt=${AES_DECRYPT(${KEY},${crypt})})
        same => n,GotoIf($[ "${original}" = "${decrypt}" ]?ok)
        same => n,Log(NOTICE,TEST is FAIL - orig <${original}> crypt <${crypt}> decrypt <${decrypt}>)
        same => n,ContinueWhile()
        same => n(ok),Log(NOTICE,TEST is GOOD - orig <${original}> crypt <${crypt}> decrypt <${decrypt}>)
    same => n,EndWhile()
    same => n,Hangup()
{noformat}

experiencing a consistent number of failures. Due to their nature, I looked at the code for {{func_aes}} and supposedly found the problem in the misuse of {{strlen}}. This is used to get the length of the data to encode in base64 in {{AES_ENCRYPT}}, but the data therein is binary, thus {{strlen}} will underestimate it at the first NULL character found:

{code}
ast_base64encode(buf, (unsigned char *) tmp, strlen(tmp), len);
{code}

A more correct estimation, IMHO, can be obtained by evaluating the offset reached by the write pointer

{code}
ast_base64encode(buf, (unsigned char *) tmp, tmpP - tmp, len);
{code}

code patched this way seems to work fine when stressed test with random data. I am submitting the patch for code review on Gerrit.

As a fun fact, similar pitfalls were discussed in the original code review at https://reviewboard.asterisk.org/r/128/.



--
This message was sent by Atlassian JIRA
(v6.2#6252)