possible rijndael bug

[Lev Walkin]

Hajimu UMEMOTO wrote:
> Hi,
> 
> 
>>>>>>On Wed, 17 Sep 2003 11:25:44 +0400 (MSD)
>>>>>>zevlg at yandex.ru ("lg") said:
> 
> 
> zevlg> I recently examined rijndael implementation, which ships in sys/crypto/rijndael and there
> zevlg> is code in function rijndael_padEncrypt()(from rijndael-api-fst.c):
> 
> zevlg> numBlocks = inputOctets/16;
> zevlg> ...
> zevlg> ...
> zevlg> padLen = 16 - (inputOctets - 16*numBlocks);
> zevlg> if (padLen > 0 && padLen <= 16)
> zevlg>         panic("...");
> zevlg> bcopy(input, block, 16 - padLen);
> zevlg> for (cp = block + 16 - padLen; cp < block + 16; cp++)
> zevlg> 	*cp = padLen;
> zevlg> rijndaelEncrypt(block, outBuffer, key->keySched, key->ROUNDS);
> zevlg> ...
> 
> zevlg> so padLen check will always success and it surely will panic, or even if we admit that 
> zevlg> padLen check is bypassed(what is impossible i think) then bcopy() will be called with 
> zevlg> larger size argument then size of block array or with negative size. Isn't this padLen 
> zevlg> check is unneeded? or maybe it should look like 'if (padLen <= 0 || padLen > 16)'?
> 
> I saw it during working on next KAME merge into 5-CURRENT.
> KAME/NetBSD uses assert() here like:
> 
> 	assert(padLen > 0 && padLen <= 16);
> 
> Since FreeBSD doesn't have assert() in kernel, this line was changed
> to:
> 
> 	if (padLen > 0 && padLen <= 16)
> 		return BAD_CIPHER_STATE;
> 
> for KAME/FreeBSD.  Since if expression is true, the assert() macro
> does nothing, the expression seems wrong, and it should be:
> 
> 	if (padLen <= 0 || padLen > 16)
> 		return BAD_CIPHER_STATE;
> 
> as you pointed out.


Absolutely NOT.

According to RFC1423 and FIPS81, the padding length may be somewhere
in between 1 to 16 bytes, which translated into

	if(padLen < 0 || padLen >= 16)

for this particular code.


-- 
Lev Walkin
vlm at netli.com