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

Scanning a QRcode that does not include an address component crashes #44

Closed
mikehearn opened this issue Feb 7, 2014 · 8 comments
Closed

Comments

@mikehearn
Copy link

  1. Go to Gavin's generator and create a bitcoin URI. It will look like:

bitcoin:?r=http%3A%2F%2Fbitcoincore.org%2F%7Egavin%2Ff.php%3Fh%3D27b38af94a7ad733d329f7302a107076

  1. Turn this into a QR code

  2. Scan. Crash due to BitcoinURI.getAddress() returning null and then getNetworkParameters() being invoked on it. In the case where no address is present, the code should just assume the URL matches the current network params and catch the mismatch later once it's downloaded.

@schildbach
Copy link
Collaborator

BIP21 formatted QR codes require an address. Even if I extend it with BIP72, I will continue requiring an address (and an amount, if present in the payment request) in order to link the URL to the payment request. For backwards compatibility I expect services to use these fields anyway.

@mikehearn
Copy link
Author

That's not compliant with the spec, which says

If the "r" parameter is provided and backwards compatibility is not required, then the bitcoin address portion of the URI may be omitted (the URI will be of the form: bitcoin:?r=... ).

That means it's up to the creator of the URI if backwards compatibility is required or not, not the wallet author.

I don't understand what you mean by "link the URL to the payment request"? If a payment request is present the data in the URI is ignored. At any rate, crashing with a NPE is definitely the wrong thing to do.

@schildbach
Copy link
Collaborator

We talked about the linking several times. The goal is I can be sure the payment request is from the person who showed me the QR code. Since up to now no better solution was agreed on, I will use what I have.

The spec is "draft" for a reason.

@mikehearn
Copy link
Author

Why would someone show you a QR code with someone else's payment request on it?

@schildbach
Copy link
Collaborator

The "request for the payment request" can be MITM'ed. X.509 is of no value for face to face payments.

@mikehearn
Copy link
Author

Sure, I'm not saying generate addressless qr codes yourself. Just don't
crash when reading them :)
On 7 Feb 2014 18:48, "Andreas Schildbach" notifications@github.com wrote:

The "request for the payment request" can be MITM'ed. X.509 is of no value
for face to face payments.


Reply to this email directly or view it on GitHubhttps://github.com/schildbach/bitcoin-wallet/issues/44#issuecomment-34480251
.

@schildbach
Copy link
Collaborator

Sure, I'm with you on that. I will fix this crash shortly.

@schildbach
Copy link
Collaborator

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants