Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
Jun 16, 2021
Merged

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

merged 1 commit into from
Jun 16, 2021

Conversation

sipa
Copy link
Contributor

@sipa sipa commented Jan 24, 2021 •
edited
Loading

Doing so could be considered UB in a pedantic interpretation of the standard. Avoid it.

Closes #876.

@sipa
Avoid passing out-of-bound pointers to 0-size memcpy
9570f67 
Doing so could be considered UB in a strict reading of the standard.
Avoid it.
@practicalswift
Copy link
Contributor

cr ACK 9570f67: patch looks correct

@real-or-random
Copy link
Contributor

real-or-random commented Jan 24, 2021 •
edited
Loading

When we change the contrib functions, we should also consider changing the copies in Bitcoin Core, see #781
edit: What I'm saying is: Let's take care of this in #781.

Can you also change the comparison in the same function here

secp256k1/src/ecdsa_impl.h

Line 115 in 9570f67

if (rlen == 0 || *sig + rlen > sigend) {

to read rlen > (size_t)(sigend - *sig) ? This will never compute an out-of-bounds pointer (which is another case of pedantic UB).

@real-or-random
Copy link
Contributor

@sipa ping

@real-or-random
Copy link
Contributor

ACK 9570f67

I'll open a new PR for the other issue

@real-or-random real-or-random merged commit 8ae56e3 into bitcoin-core:master Jun 16, 2021
@real-or-random real-or-random mentioned this pull request Jun 16, 2021
Merged
@apoelstra apoelstra mentioned this pull request Jul 27, 2021
Merged
real-or-random added a commit that referenced this pull request Oct 17, 2021
@real-or-random
Merge #952: Avoid computing out-of-bounds pointer.
920a0e5 
9be7b0f Avoid computing out-of-bounds pointer. (Tim Ruffing)

Pull request description:

  This is a pedantic case of UB.

  Spotted in #879.

ACKs for top commit:
  elichai:
    ACK 9be7b0f
  practicalswift:
    cr ACK 9be7b0f
  sipa:
    ACK 9be7b0f

Tree-SHA512: a9d028c4cdb37ad0d5fcf0d2f678eef732a653d37155a69a20272c6b283c28e083172485d7a37dc4a7c6100b22a6f5b6a92e729239031be228cc511842ee35e8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Potential UB in secp256k1_der_parse_integer?
3 participants
@sipa @practicalswift @real-or-random