Skip to content

Fix crash in validateaddress with -disablewallet #6970

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
Merged
laanwj merged 1 commit into from
Nov 9, 2015
Merged

Fix crash in validateaddress with -disablewallet #6970

laanwj merged 1 commit into from
Nov 9, 2015

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Nov 9, 2015

Fix a null pointer dereference in validateaddress with -disablewallet. Also add a regression testcase.

Problem reported here: #6963 (comment)

I think this needs to be backported to 0.11 as well.

@laanwj
Fix crash in validateaddress with -disablewallet
2980a18 
Fix a null pointer dereference in validateaddress with -disablewallet. Also add a regression testcase.
@jonasschnelli
Copy link
Contributor

jonasschnelli commented Nov 9, 2015

Nice catch! I was also looking for the NULL pointer access... we need to be careful by always keeping in mind: #ifdef ENABLE_WALLET != pwalletMain available.

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Nov 9, 2015

utACK

@paveljanik
Copy link
Contributor

paveljanik commented Nov 9, 2015

ACK
Checked other instances, OK.

@dcousens
Copy link
Contributor

dcousens commented Nov 9, 2015

utACK

@fanquake
Copy link
Member

fanquake commented Nov 9, 2015

utACK

On Monday, November 9, 2015, Daniel Cousens [email protected]
wrote:

utACK

—
Reply to this email directly or view it on GitHub
#6970 (comment).

@maflcko
Copy link
Member

maflcko commented Nov 9, 2015

backported to 0.11

yes and utACK.

@laanwj laanwj merged commit 2980a18 into bitcoin:master Nov 9, 2015
laanwj added a commit that referenced this pull request Nov 9, 2015
@laanwj
Merge pull request #6970
92701b3 
2980a18 Fix crash in validateaddress with -disablewallet (Wladimir J. van der Laan)
@laanwj
Copy link
Member Author

laanwj commented Nov 9, 2015

Good news: the issue doesn't exist in 0.11 (and hence not in any released version)

This problem was introduced in 506bae3 (#6262) by @dexX7 . The code used to check for ISMINE_*, which can only be ISMINE_NO if there is no wallet, but that check was removed there.

@jgarzik
Copy link
Contributor

jgarzik commented Nov 9, 2015

ut ACK

@jlopp
Copy link
Contributor

jlopp commented Nov 9, 2015

Thanks for the quick fix!

@laanwj
Copy link
Member Author

laanwj commented Nov 9, 2015

@jlopp Did it fix your issue? (I still don't see how this can cause a SIGILL in OpenSSL)

@jlopp
Copy link
Contributor

jlopp commented Nov 9, 2015

@laanwj Yes, I confirmed that my node no longer crashes after issuing the validateaddress command. As for the other errors I had posted in my comments, those were errors that the debugger was throwing immediately upon trying to run bitcoind, not upon calling validateaddress.

luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Nov 18, 2015
@laanwj @luke-jr
rpc-tests: Backport test of validateaddress with -disablewallet
1b0fde1 
Github-Pull: bitcoin#6970
Rebased-From: 2980a18
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Dec 8, 2015
@laanwj @luke-jr
rpc-tests: Backport test of validateaddress with -disablewallet
312cf15 
Github-Pull: bitcoin#6970
Rebased-From: 2980a18
@crowning- crowning- mentioned this pull request Feb 14, 2016
Closed
@daira daira mentioned this pull request Feb 26, 2017
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Mar 4, 2017
@zkbot
Auto merge of #2100 - str4d:2074-rpc, r=arcalinea
a7cf698 
Bitcoin 0.12 RPC PRs 1

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6266
- bitcoin/bitcoin#6257
- bitcoin/bitcoin#6271
- bitcoin/bitcoin#6158
- bitcoin/bitcoin#6307
- bitcoin/bitcoin#6290
- bitcoin/bitcoin#6262
- bitcoin/bitcoin#6088
- bitcoin/bitcoin#6339
- bitcoin/bitcoin#6299 (partial, remainder in #2099)
- bitcoin/bitcoin#6350
- bitcoin/bitcoin#6247
- bitcoin/bitcoin#6362
- bitcoin/bitcoin#5486
- bitcoin/bitcoin#6417
- bitcoin/bitcoin#6398 (partial, remainder was included in #1950)
- bitcoin/bitcoin#6444
- bitcoin/bitcoin#6456 (partial, remainder was included in #2082)
- bitcoin/bitcoin#6380
- bitcoin/bitcoin#6970

Part of #2074.
@dagurval dagurval mentioned this pull request Jan 17, 2018
Merged
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Bug RPC/REST/ZMQ

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@laanwj @jonasschnelli @paveljanik @dcousens @fanquake @maflcko @jgarzik @jlopp