1227090 - [Static Analysis][Division by zero] Function makePfromQandSeed from lib/freebl/pqg.c could lead to a division by 0

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[Andi [:andi]]

The Static Analysis tool Scan-Build added that variable outlen from makePfromQandSeed could lead to a division by 0 if HASH_ResultLen gets called with improper parameters.

Comment 1

[Andi [:andi]]

Attachment: Bug 1227090.diff

Comment 2

[Andi [:andi]]

Attachment: Bug 1227090.diff

Updated previous path in order to log the error with invalid argument function call.

Comment 3

[Andi [:andi]]

Attachment: Bug 1227090.diff

Hello Robert,

Could you please take a look other this patch?

THX

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8696423 [details] [diff] [review]
Bug 1227090.diff

Review of attachment 8696423 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/nss/lib/freebl/pqg.c
@@ +1009,5 @@
>      CHECK_MPI_OK( mp_init(&V_n)  );
>  
>      hashlen = HASH_ResultLen(hashtype);
>      outlen = hashlen*PR_BITS_PER_BYTE;
> +    if (outlen <= 0)

This should rather be an assertion. There's no way that `makePfromQandSeed()` is passed an invalid hash type.

Comment 5

[Andi [:andi]]

Attachment: Bug 1227090.patch

Comment 6

[Ludovic Hirlimann [:Usul]]

:andi Tim is gone, maybe ask some else to review these?

Comment 7

[Andi [:andi]]

Attachment: Bug 1227090 - assert if outlen is greater than 0. r=kaie

Comment 8

[J.C. Jones [:jcj] (he/him)]

Try run: https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=b1ad02d3b635

Comment 9

[J.C. Jones [:jcj] (he/him)]

https://hg.mozilla.org/projects/nss/rev/b9bfa1d564f2f88d979d804f29b3bb0c555d3581