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

[Asterisk Team (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-25857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=229957#comment-229957 ] 

Asterisk Team commented on ASTERISK-25857:
------------------------------------------

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].

> 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: 11.21.2, 13.6.0, 13.7.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)