The static analyser found few minor issues in libgcrypt. I opened a MR last week but sharing also here.
https://gitlab.com/redhat-crypto/libgcrypt/libgcrypt-mirror/-/merge_requests/18/
The analysis was executed on 1.10.3
Description
Description
Details
Details
- Version
- master, 1.10.3
Revisions and Commits
Revisions and Commits
| rC libgcrypt | |||
| rCdc8d84383a6b cipher:aeswrap: Fix padding length check. | |||
| rC187575844015 mpi: Fix loop condition in bad point check. | |||
Related Objects
Related Objects
- Mentioned In
- T7165: Release Libgcrypt 1.11.0
Event Timeline
Comment Actions
Can you give a hint where there is a buffer overrun in the first patch? Padding limit might be correct but I can't see an overrun.
Comment Actions
The report went like this
Error: OVERRUN (CWE-119):
libgcrypt-1.10.3/cipher/cipher-aeswrap.c:303: cond_at_most: Checking "plen > 8U" implies that "plen" may be up to 8 on the false branch.
libgcrypt-1.10.3/cipher/cipher-aeswrap.c:305: cond_between: Checking "plen" implies that "plen" is between 1 and 8 (inclusive) on the true branch.
libgcrypt-1.10.3/cipher/cipher-aeswrap.c:309: assignment: Assigning: "i" = "0".
libgcrypt-1.10.3/cipher/cipher-aeswrap.c:310: overrun-local: Overrunning array "t" of 16 bytes at byte offset 16 using index "8U + plen + i" (which evaluates to 16).
# 308|
# 309| for (i = 0; i < 16 - (8+plen); i++)
# 310|-> if (t[8+plen+i])
# 311| {
# 312| err = GPG_ERR_CHECKSUM;but looking again, it is wrong as it did not reflect the end condition for the cycle, which obviously means the cycle does not run. Sorry for the noise.
Comment Actions
Well, but if the padding is indeed limited to 7 bytes the fix should be applied anyway.
Comment Actions
Right, thats my understanding from reading of the RFC that the padding should be strictly < 8B. We can reword though.