rsaenh: fix buffer overrun in RSAENH_CPVerifySignature and add parameters validity checks in RSAENH_CPEncrypt, RSAENH_CPDecrypt and RSAENH_CPSignHash

[Mounir IDRASSI]

Hi,
This patch fixes the following issues :
    - RSAENH_CPVerifySignature: the signature length was not checked.
This leads to a buffer overrun when the signature is smaller than the
key length.
    - RSAENH_CPEncrypt and RSAENH_CPSignHash: some parameters are not
validated before being used. This leads to crashes when invalid input is
given.
    - RSAENH_CPDecrypt: some parameters are not validated before being
used. Padding in case of block ciphers was removed whithout beeing
checked: In case of corrupted ciphered input, no error is reported and
output data length is set to an invalid value.

Mounir IDRASSI
IDRIX - Cryptography and IT Security Experts
http://www.idrix.fr

-------------- next part --------------
>From 346e62d41d03aa546ba6fa66db38638812026706 Mon Sep 17 00:00:00 2001
From: Mounir IDRASSI <mounir.idrassi at idrix.fr>
Date: Sat, 12 May 2007 18:04:45 +0200
Subject: rsaenh: fix buffer overrun in RSAENH_CPVerifySignature and add parameters validity checks in RSAENH_CPEncrypt, RSAENH_CPDecrypt and RSAENH_CPSignHash

---
 dlls/rsaenh/rsaenh.c |  103 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 102 insertions(+), 1 deletions(-)

diff --git a/dlls/rsaenh/rsaenh.c b/dlls/rsaenh/rsaenh.c
index 3e7ac8e..1857b1b 100644
--- a/dlls/rsaenh/rsaenh.c
+++ b/dlls/rsaenh/rsaenh.c
@@ -1883,6 +1883,12 @@ BOOL WINAPI RSAENH_CPEncrypt(HCRYPTPROV hProv, HCRYPTKEY hKey, HCRYPTHASH hHash,
         return FALSE;
     }
 
+    if (!pdwDataLen)
+    {
+        SetLastError(ERROR_INVALID_PARAMETER);
+        return FALSE;
+    }
+
     if (pCryptKey->dwState == RSAENH_KEYSTATE_IDLE) 
         pCryptKey->dwState = RSAENH_KEYSTATE_ENCRYPTING;
 
@@ -2035,6 +2041,12 @@ BOOL WINAPI RSAENH_CPDecrypt(HCRYPTPROV hProv, HCRYPTKEY hKey, HCRYPTHASH hHash,
         return FALSE;
     }
 
+    if (!pdwDataLen)
+    {
+        SetLastError(ERROR_INVALID_PARAMETER);
+        return FALSE;
+    }
+
     if (pCryptKey->dwState == RSAENH_KEYSTATE_IDLE) 
         pCryptKey->dwState = RSAENH_KEYSTATE_DECRYPTING;
 
@@ -2047,6 +2059,32 @@ BOOL WINAPI RSAENH_CPDecrypt(HCRYPTPROV hProv, HCRYPTKEY hKey, HCRYPTHASH hHash,
     dwMax=*pdwDataLen;
 
     if (GET_ALG_TYPE(pCryptKey->aiAlgid) == ALG_TYPE_BLOCK) {
+        if (dwMax % pCryptKey->dwBlockLen)
+        {
+            SetLastError(NTE_BAD_DATA);
+            return FALSE;
+        }
+        
+        if (Final)
+        {
+            if (!dwMax)
+            {
+                SetLastError(NTE_BAD_LEN);
+                return FALSE;
+            }
+
+            if (!pbData)
+            {
+                SetLastError(NTE_BAD_DATA);
+                return FALSE;
+            }
+        }
+        else if (!pbData && dwMax)
+        {
+            SetLastError(ERROR_INVALID_PARAMETER);
+            return FALSE;
+        }
+
         for (i=0, in=pbData; i<*pdwDataLen; i+=pCryptKey->dwBlockLen, in+=pCryptKey->dwBlockLen) {
             switch (pCryptKey->dwMode) {
                 case CRYPT_MODE_ECB:
@@ -2078,7 +2116,33 @@ BOOL WINAPI RSAENH_CPDecrypt(HCRYPTPROV hProv, HCRYPTKEY hKey, HCRYPTHASH hHash,
             }
             memcpy(in, out, pCryptKey->dwBlockLen);
         }
-        if (Final) *pdwDataLen -= pbData[*pdwDataLen-1]; 
+        
+        if (Final)
+        {
+            /* check that the padding is correct */
+            LPBYTE pbPaddedData = &pbData[*pdwDataLen-1];
+            BYTE bPadByte = *pbPaddedData;
+
+            if (bPadByte > 0x08)
+            {
+                SetLastError(NTE_BAD_DATA);
+                return FALSE;
+            }
+
+            i = 1;
+            while (i < bPadByte)
+            {    
+                pbPaddedData--;
+                if (*pbPaddedData != bPadByte)
+                {
+                    SetLastError(NTE_BAD_DATA);
+                    return FALSE;
+                }
+                i++;
+            }
+            /* everything is ok. write the output length*/    
+            *pdwDataLen -= bPadByte;
+        }
 
     } else if (GET_ALG_TYPE(pCryptKey->aiAlgid) == ALG_TYPE_STREAM) {
         encrypt_stream_impl(pCryptKey->aiAlgid, &pCryptKey->context, pbData, *pdwDataLen);
@@ -2087,6 +2151,25 @@ BOOL WINAPI RSAENH_CPDecrypt(HCRYPTPROV hProv, HCRYPTKEY hKey, HCRYPTHASH hHash,
             SetLastError(NTE_BAD_KEY);
             return FALSE;
         }
+
+        if (!dwMax)
+        {
+            SetLastError(NTE_BAD_LEN);
+            return FALSE;
+        }
+
+        if (!pbData)
+        {
+            SetLastError(ERROR_INVALID_PARAMETER);
+            return FALSE;
+        }
+
+        if (dwMax != pCryptKey->dwBlockLen)
+        {
+            SetLastError(NTE_BAD_DATA);
+            return FALSE;
+        }
+
         encrypt_block_impl(pCryptKey->aiAlgid, PK_PRIVATE, &pCryptKey->context, pbData, pbData, RSAENH_DECRYPT);
         if (!unpad_data(pbData, pCryptKey->dwBlockLen, pbData, pdwDataLen, dwFlags)) return FALSE;
         Final = TRUE;
@@ -3526,6 +3609,12 @@ BOOL WINAPI RSAENH_CPSignHash(HCRYPTPROV hProv, HCRYPTHASH hHash, DWORD dwKeySpe
         return FALSE;
     }
 
+    if (!hHash)
+    {
+        SetLastError(ERROR_INVALID_PARAMETER);
+        return FALSE;
+    }
+
     if (!pbSignature) {
         *pdwSigLen = pCryptKey->dwKeyLen;
         return TRUE;
@@ -3611,6 +3700,18 @@ BOOL WINAPI RSAENH_CPVerifySignature(HCRYPTPROV hProv, HCRYPTHASH hHash, CONST B
         return FALSE;
     }
 
+    if (!hHash || !pbSignature)
+    {
+        SetLastError(ERROR_INVALID_PARAMETER);
+        return FALSE;
+    }
+
+    if (dwSigLen != pCryptKey->dwKeyLen)
+    {
+        SetLastError(NTE_BAD_SIGNATURE);
+        return FALSE;
+    }
+
     if (sDescription) {
         if (!RSAENH_CPHashData(hProv, hHash, (CONST BYTE*)sDescription, 
                                 (DWORD)lstrlenW(sDescription)*sizeof(WCHAR), 0))
-- 
1.4.4.2