Avoid computing out-of-bounds pointer. #952

1. 
   real-or-random commented at 8:36 am on June 16, 2021: contributor

   This is a pedantic case of UB.

   Spotted in #879.

2. Avoid computing out-of-bounds pointer.

   This is a pedantic case of UB.

   9be7b0f083
3. 
   practicalswift commented at 12:33 pm on June 16, 2021: contributor

   Concept ACK
4. 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.)

5. 
   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
6. 
   real-or-random commented at 1:53 pm on October 15, 2021: contributor

   Concept ACK @practicalswift are you willing to review this in detail?

7. 
   practicalswift commented at 9:46 am on October 16, 2021: contributor

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

   cr ACK 9be7b0f08340a063d961547b5d2663405f3fc162

8. 
   sipa commented at 2:45 am on October 17, 2021: contributor

   ACK 9be7b0f08340a063d961547b5d2663405f3fc162
9. real-or-random merged this on Oct 17, 2021
10. real-or-random closed this on Oct 17, 2021

    ---

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

Contributors
real-or-random practicalswift elichai sipa