Date: Wed, 17 Sep 2003 01:09:24 -0700 From: Lev Walkin <vlm@netli.com> To: Hajimu UMEMOTO <ume@freebsd.org> Cc: core@kame.net Subject: Re: possible rijndael bug Message-ID: <3F6816B4.10607@netli.com> In-Reply-To: <ygepthz7sr7.wl%ume@bisd.hitachi.co.jp> References: <3F680C78.000003.13537@tide.yandex.ru> <ygepthz7sr7.wl%ume@bisd.hitachi.co.jp>
next in thread | previous in thread | raw e-mail | index | archive | help
Hajimu UMEMOTO wrote:
> Hi,
>
>
>>>>>>On Wed, 17 Sep 2003 11:25:44 +0400 (MSD)
>>>>>>zevlg@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@netli.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3F6816B4.10607>