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

Fix crash in validateaddress with -disablewallet #6970

Merged
merged 1 commit into from Nov 9, 2015

Conversation

@laanwj
Copy link
Member

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.

Fix a null pointer dereference in validateaddress with -disablewallet. Also add a regression testcase.
@jonasschnelli
Copy link
Member

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
Member

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 notifications@github.com
wrote:

utACK


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

@MarcoFalke
Copy link
Member

MarcoFalke commented Nov 9, 2015

backported to 0.11

yes and utACK.

@laanwj laanwj merged commit 2980a18 into bitcoin:master Nov 9, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Nov 9, 2015
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 added a commit to luke-jr/bitcoin that referenced this pull request Nov 18, 2015
luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Dec 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.