[jira] [Updated] (SANTUARIO-400) Buffer overwrite in WinCAPICryptoSymmetricKey::encrypt() (WinCAPICryptoSymmetricKey.cpp)

["Scott Cantor (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SANTUARIO-400?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Scott Cantor updated SANTUARIO-400:
-----------------------------------
    Fix Version/s:     (was: C++ 1.7.2)

> Buffer overwrite in WinCAPICryptoSymmetricKey::encrypt() (WinCAPICryptoSymmetricKey.cpp)
> ----------------------------------------------------------------------------------------
>
>                 Key: SANTUARIO-400
>                 URL: https://issues.apache.org/jira/browse/SANTUARIO-400
>             Project: Santuario
>          Issue Type: Bug
>          Components: C++
>    Affects Versions: C++ 1.7.0, C++ 1.7.1, C++ 1.7.2
>         Environment: Windows 7, VS2010 SP1
>            Reporter: Serge Lalonde
>            Assignee: Scott Cantor
>            Priority: Critical
>
> If the size of an Xml file being encrypted is 2KB + N, where N is a value less than m_blockSize, WinCAPICryptoSymmetricKey::encrypt() will generate a buffer overwrite at two locations, the second of which will cause a crash. Note that the 2 KB is the input buffer size.
> This happens because of a signed/unsigned overflow because the local variable "rounding" is greater than the method argument "inLength".
> Here are the locations where the overwrites occur:
> 1. line 558
>    memcpy(m_lastBlock, &inBuf[inLength - rounding], rounding);
> 2. line 562 (causes a crash)
>    memcpy(bufPtr, inBuf, inLength - rounding);
> There is a comment in the code that references that a fix was added to this method for bug 38365, which references very small inputs (which I presume means very small Xml files). Here's the patch in question:
> 	/* As per bug #38365 - we need to ensure there are enough characters to encrypt.
> 	   Otherwise we get some nasty errors due to rounding calculations when inputs
> 	   are too small */
> 	if (m_bytesInLastBlock + inLength < m_blockSize) {
> 		// Not enough input data
> 		memcpy(&m_lastBlock[m_bytesInLastBlock], inBuf, inLength);
> 		m_bytesInLastBlock += inLength;
> 		// Just in case we have returned an IV
> 		return outl;
> 	}
> In fact, this fix can be generalized easily to fix my problem also, since the if condition should really be
> 	if (inLength < m_blockSize) {
> since for very small inputs, m_bytesInLastBlock will be 0 anyway. This change allow the early return to be executed in both instances and avoids the overflow which gives a very large number and causes the crash.
> I've tested this fix in 1.7.2 and it works properly. If it could get implemented in the main code branch, that would be great.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)