A possible divide by zero bug in drbg_ctr_df

A possible divide by zero bug in drbg_ctr_df

[Yiyuan guo]

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?

Re: A possible divide by zero bug in drbg_ctr_df

[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?

Add Stephan to the cc as he is the author.

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

Re: A possible divide by zero bug in drbg_ctr_df

[Stephan Mueller]

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