Date: Wed, 17 Sep 2003 00:43:51 -0700 From: Lev Walkin <vlm@netli.com> To: zevlg@yandex.ru Cc: hackers@freebsd.org Subject: Re: possible rijndael bug Message-ID: <3F6810B7.4050205@netli.com> In-Reply-To: <3F680C78.000003.13537@tide.yandex.ru> References: <3F680C78.000003.13537@tide.yandex.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
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@netli.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3F6810B7.4050205>