possible rijndael bug

[Hajimu UMEMOTO]

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.

Sincerely,

--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
ume at mahoroba.org  ume at bisd.hitachi.co.jp  ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/