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.

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

Add length check for CExtKey deserialization (jonasschnelli, guidovranken) #11081

Merged
merged 1 commit into from Aug 18, 2017

Conversation

Projects
None yet
4 participants
@jonasschnelli
Copy link
Member

commented Aug 17, 2017

Fix a potential overwrite or uninitialised data issue.
That code part is currently unused (at least in Bitcoin Core).
We already do the same check CExtPubKey.

Reported by @guidovranken

@laanwj laanwj added the Wallet label Aug 17, 2017

@laanwj

This comment has been minimized.

Copy link
Member

commented Aug 17, 2017

utACK

We should probably credit the person who reported this?

@practicalswift

This comment has been minimized.

Copy link
Member

commented Aug 18, 2017

Ouch! Really nice find! Found by code review or fuzzing?

utACK 07685d1

@jonasschnelli

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2017

Issue was discovered by Guido Vranken via Fuzzing (@guidovranken / see also #11045).
It looks like non of the Core forks is using this as well.

@practicalswift

This comment has been minimized.

Copy link
Member

commented Aug 18, 2017

Thank you @guidovranken!

@MarcoFalke MarcoFalke changed the title Add length check for CExtKey deserialization Add length check for CExtKey deserialization (jonasschnelli, guidovranken) Aug 18, 2017

@laanwj laanwj added this to the 0.15.0 milestone Aug 18, 2017

@laanwj laanwj merged commit 07685d1 into bitcoin:master Aug 18, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Aug 18, 2017

Merge #11081: Add length check for CExtKey deserialization (jonasschn…
…elli, guidovranken)

07685d1 Add length check for CExtKey deserialization (Jonas Schnelli)

Pull request description:

  Fix a potential overwrite or uninitialised data issue.
  That code part is currently unused (at least in Bitcoin Core).
  We already do the same check `CExtPubKey`.

  Reported by @guidovranken

Tree-SHA512: 069ac5335248cf890491bc019537d3b0f7481428a4b240c5cd28ee89b56f4c9f45d947dd626fe89b2fae58472b6dbef57ed909876efe9963e2d72380d17cff12
@guidovranken

This comment has been minimized.

Copy link

commented Aug 18, 2017

You're welcome.

laanwj added a commit that referenced this pull request Aug 21, 2017

Add length check for CExtKey deserialization
Github-Pull: #11081
Rebased-From: 07685d1
Tree-SHA512: 7f6b9ca6714d059d500531eb842c1c25edfa04ecba0ea1d9a28010fced657c066cea67d2016bbaa9b96c431a05ca9c0dcf2ba301898ecf96a65a4e01aac7fae9

@laanwj laanwj removed the Needs backport label Aug 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.