2021-05-21 09:33:48

by Yiyuan Guo

Subject: A possible divide by zero bug in drbg_ctr_df

In crypto/drbg.c, the function drbg_ctr_df has the following code:

padlen = (inputlen + sizeof(L_N) + 1) % (drbg_blocklen(drbg));

However, the function drbg_blocklen may return zero:

static inline __u8 drbg_blocklen(struct drbg_state *drbg)
{
if (drbg && drbg->core)
return drbg->core->blocklen_bytes;
return 0;
}

Is it possible to trigger a divide by zero problem here?

2021-05-21 09:58:46

by Herbert Xu

Subject: Re: A possible divide by zero bug in drbg_ctr_df

On Fri, May 21, 2021 at 11:23:36AM +0800, Yiyuan guo wrote:
> In crypto/drbg.c, the function drbg_ctr_df has the following code:
>
> padlen = (inputlen + sizeof(L_N) + 1) % (drbg_blocklen(drbg));
>
> However, the function drbg_blocklen may return zero:
>
> static inline __u8 drbg_blocklen(struct drbg_state *drbg)
> {
> if (drbg && drbg->core)
> return drbg->core->blocklen_bytes;
> return 0;
> }
>
> Is it possible to trigger a divide by zero problem here?

Add Stephan to the cc as he is the author.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2021-05-21 10:09:40

by Stephan Müller

Subject: Re: A possible divide by zero bug in drbg_ctr_df

Am Freitag, dem 21.05.2021 um 14:48 +0800 schrieb Herbert Xu:
> On Fri, May 21, 2021 at 11:23:36AM +0800, Yiyuan guo wrote:
> > In crypto/drbg.c, the function drbg_ctr_df has the following code:
> >
> > padlen = (inputlen + sizeof(L_N) + 1) % (drbg_blocklen(drbg));
> >
> > However, the function drbg_blocklen may return zero:
> >
> > static inline __u8 drbg_blocklen(struct drbg_state *drbg)
> > {
> >     if (drbg && drbg->core)
> >         return drbg->core->blocklen_bytes;
> >     return 0;
> > }
> >
> > Is it possible to trigger a divide by zero problem here?


I do not think there is a problem. Allow me to explain:

To reach the drbg_ctr_df function, the drbg_ctr_update function must be
called. This is either called from the seeding operation or the generate
operation.

The seeding operation is guarded as follows:

1. if called from the instantiation drbg_instantiate, we have:

if (!drbg->core) {
drbg->core = &drbg_cores[coreref];

2. if called from the generate function drbg_generate, we have:

if (!drbg->core) {
pr_devel("DRBG: not yet seeded\n");
return -EINVAL;
}

Thus, in both cases, when no drbg and no drbg->core is present, either the
code tries to get it or it fails before trying to invoke the concerning code
path.


When the drbg_ctr_update function is invoked from the generate operation, the
step 2 above applies.


Thus, when we reach the call for drbg_blocklen to get the padlen, we always
have a drbg and a drbg->core pointer.

In general, as soon as the DRBG code path reaches the DRBG-specific
implementations hiding behind drbg->[update|generate], the entire DRBG is
fully initialized and all pointers/memory is set up as needed.

The sanity check in drbg_blocklen is there as the function may be called in
earlier functions where it is not fully guaranteed that the drbg and drbg-
>core is set.

Thanks for the review.
Stephan