Avoid computing out-of-bounds pointer. #952

pull real-or-random wants to merge 1 commits into bitcoin-core:master from real-or-random:202106-oob-pointer changing 1 files +1 −1

real-or-random commented at 8:36 AM on June 16, 2021: contributor

This is a pedantic case of UB.

Spotted in #879.
Avoid computing out-of-bounds pointer.
This is a pedantic case of UB.
9be7b0f083
practicalswift commented at 12:33 PM on June 16, 2021: contributor

Concept ACK
in src/ecdsa_impl.h:115 in 9be7b0f083
```
 111 | @@ -112,7 +112,7 @@ static int secp256k1_der_parse_integer(secp256k1_scalar *r, const unsigned char
 112 |      if (secp256k1_der_read_len(&rlen, sig, sigend) == 0) {
 113 |          return 0;
 114 |      }
 115 | -    if (rlen == 0 || *sig + rlen > sigend) {
 116 | +    if (rlen == 0 || rlen > (size_t)(sigend - *sig)) {
```
elichai commented at 12:21 PM on September 2, 2021:

nit, maybe casting rlen to ptrdiff_t is more "correct" (in the standard purist sense)?

real-or-random commented at 10:57 PM on September 2, 2021:

It's UB if that cast to ptrdiff_t overflows, and rlen is attacker-controlled data.

(We have these size_t casts everywhere in the file.)
elichai commented at 4:36 PM on September 3, 2021: contributor

ACK 9be7b0f08340a063d961547b5d2663405f3fc162 This should be correct because we know that signend is always bigger or equal to *sig, so the result of the subtraction must always yield a positive integer
real-or-random commented at 1:53 PM on October 15, 2021: contributor

Concept ACK @practicalswift are you willing to review this in detail?
practicalswift commented at 9:46 AM on October 16, 2021: contributor

@real-or-random Sure! Thanks for the ping! :)

cr ACK 9be7b0f08340a063d961547b5d2663405f3fc162
sipa commented at 2:45 AM on October 17, 2021: contributor

ACK 9be7b0f08340a063d961547b5d2663405f3fc162
real-or-random merged this on Oct 17, 2021
real-or-random closed this on Oct 17, 2021
real-or-random deleted the branch on Oct 17, 2021
fanquake referenced this in commit 8f5cd5e893 on Oct 20, 2021
sipa referenced this in commit f727914d7e on Oct 28, 2021
sipa referenced this in commit 440f7ec80e on Oct 31, 2021
sipa referenced this in commit d057eae556 on Dec 2, 2021
fanquake referenced this in commit c4a1e09a8c on Dec 3, 2021
sipa referenced this in commit 86dbc4d075 on Dec 15, 2021
jonasnick cross-referenced this on Jan 2, 2022 from issue Sync Upstream by jonasnick
real-or-random referenced this in commit 21e2d65b79 on Jan 5, 2022
gwillen referenced this in commit 35d6112a72 on May 25, 2022
janus referenced this in commit 879a9a27b9 on Jul 10, 2022
patricklodder referenced this in commit 21badcf9d2 on Jul 25, 2022
patricklodder referenced this in commit 03002a9013 on Jul 28, 2022
backpacker69 referenced this in commit 77186f4a04 on Jan 18, 2023
str4d referenced this in commit 6de4698bf9 on Apr 21, 2023
vmta referenced this in commit e1120c94a1 on Jun 4, 2023
vmta referenced this in commit 8f03457eed on Jul 1, 2023

Contributors

Linked (view graph)

#879 Avoid passing out-of-bound pointers to 0-size memcpy