possible rijndael bug

[Lev Walkin]

lg wrote:
> Hello hackers.
> 
> I recently examined rijndael implementation, which ships in sys/crypto/rijndael and there
> is code in function rijndael_padEncrypt()(from rijndael-api-fst.c):
> 
> numBlocks = inputOctets/16;
> ...
> ...
> padLen = 16 - (inputOctets - 16*numBlocks);
> if (padLen > 0 && padLen <= 16)
>         panic("...");
> bcopy(input, block, 16 - padLen);
> for (cp = block + 16 - padLen; cp < block + 16; cp++)
> 	*cp = padLen;
> rijndaelEncrypt(block, outBuffer, key->keySched, key->ROUNDS);
> ...
> 
> so padLen check will always success and it surely will panic, or even if we admit that 
> padLen check is bypassed(what is impossible i think) then bcopy() will be called with 
> larger size argument then size of block array or with negative size. Isn't this padLen 
> check is unneeded? or maybe it should look like 'if (padLen <= 0 || padLen > 16)'?
> 
> In RFC2040 there is a description about how to process last block and there is not such 
> checks.

Thanks God ECB mode isn't in much use nowadays.


 From KAME sources:


http://orange.kame.net/dev/cvsweb.cgi/kame/kame/sys/crypto/rijndael/rijndael-api-fst.c.diff?r1=1.15&r2=1.16


=== cut cvs diff ===
  		padLen = 16 - (inputOctets - 16*numBlocks);
-		if (padLen <= 0 || padLen > 16)
-			panic("rijndael_padEncrypt(ECB)");
-		bcopy(input, block, 16 - padLen);
-		for (cp = block + 16 - padLen; cp < block + 16; cp++)
-			*cp = padLen;
-		rijndaelEncrypt(block, outBuffer, key->keySched, key->ROUNDS);
+		assert(padLen > 0 && padLen <= 16);
+		memcpy(block, input, 16 - padLen);
+		memset(block + 16 - padLen, padLen, padLen);
+		rijndaelEncrypt(key->rk, key->Nr, block, outBuffer);
  		break;
=== cut cvs diff ===

And then somebody changed assert() into a direct if()!

-- 
Lev Walkin
vlm at netli.com