Disable bitcoin: URI handling on Windows for the 0.6 release #991

1. 
   gavinandresen commented at 4:24 PM on March 26, 2012: contributor

   I don't think bitcoin: URI handling on Windows has had nearly enough testing, so this disables it for the 0.6 release.

   I'm all for re-enabling it for a 0.6.1 and/or 0.7, but only we take a step back and do two things:

   1. Can we get an IPC solution that doesn't require monkey-patching boost::interprocess ?
   2. I'm nervous that attackers might be able to craft bitcoin: URIs that Do Bad Things. I would suggest that we disable Linux bitcoin: URI handling until that is fixed, but I consider bitcoin: URI handling on Linux experimental anyway because users have to know how to manually enable it.
2. Disable bitcoin: URI handling on Windows for the 0.6 release 7b90edb5a6
3. 
   TheBlueMatt commented at 5:11 PM on March 26, 2012: member

   1. #986 is a dead-simple patch which solves a known and well-documented issue in boost::interprocesses. What it does is clear and simple and there is no reason to avoid it.
   2. This one has been discussed over and over again (we've had one pull request or another sitting around getting slow discussion about the possibility of attackers hitting bitcoin with URIs since before 0.4). At this point, I have yet to hear anything remotely convincing that could indicate an attacker could do anything remotely useful to attack this.
   3. We already have to have a significant amount of time dedicated to 0.6rc5 testing before we can release, blocking URI handling on windows for 0.6 is really not the way to go. It is a very simple feature with one very minor bug (which manifests itself in several ugly ways). If there are still issues after #986 is merged in 0.6rc5, then yea Id say disable it until 0.7.
   4. I really, really believe URI support is a very important usability feature (much more important than P2SH or many of the other things we've spent serious time implementing). It offers users a much, much more user-friendly way of making payto links and gives a much more professional and clean feeling to paying online merchants (which is one of the biggest use-cases for bitcoin today).
4. 
   laanwj commented at 5:35 PM on March 26, 2012: member

   Re: URLs that do bad things, any special reason that you think this is suddenly an issue? You ACKed it first time and I don't think this aspect changed.

   Given that there are no fatal bugs in boost::interprocess and the Qt URL parser, I think it's pretty unlikely there are security problems.

5. 
   gavinandresen commented at 6:07 PM on March 26, 2012: contributor

   1. #986 is "a somewhat-hacky workaround" to a boost bug.
   2. It has been discussed over and over again, and, apparently, been essentially untested.
   3. Adding even more things to test in the 5->final release process is not a good argument.
   4. I have strong reservations about bitcoin: URIs, especially given the recent report of Javascript-based bitcoin address rewriting trojans. I think we need a better solution to the problem: "how do we confirm that we're paying who we think we're paying?"

   RE: why do I suddenly think bad URLs are an issue: because when I see stupid-simple things like the setup.nsi file launching bitcoin.exe instead of bitcoin-qt.exe getting fixed this close to a final release it drives home the point that THIS FEATURE HAS NOT BEEN SUFFICIENTLY TESTED.

   Excuse me for yelling, this just feels like the wallet encryption (mis-)feature all over again.

6. 
   sipa commented at 6:26 PM on March 26, 2012: member

   Since apparently URI's didn't work at all in windows releases so far, I really don't mind disabling URI's for now to get 0.6.0 out, and re-enable them as soon as they have actually been tested.

7. 
   laanwj commented at 6:46 PM on March 26, 2012: member

   I'm fine with disabling it for this release.

8. 
   TheBlueMatt commented at 6:47 PM on March 26, 2012: member

   1. Somewhat hacky in the sense that it doesnt bother fixing the issues with its COM usage, but instead just works around broken COM code. Just because its "hacky" doesnt mean its bad nor is it a reason to avoid it.
   2. It was very thoroughly tested aside from the ipc issues (which only cropped up in bitcoin-qt, and were not there using the same code in wx).
   3. Not really adding anything, because its already there, been tested on Linux/Windows, just caused a few bugs on Windows, which were found thanks to it being tested.
   4. Yes, js trojans can just as easily rewrite URIs as addresses, but what is the difference? It doesnt change the security but makes it easier to pay... As for how we confirm we are paying who we think we are, any sane merchant should be using SSL and sane xss protection, which all but removes any threat of js trojans (aside from browser plugins hijacking stuff). If you want to try to block that, you can try to use aliases like the old https requests but that only moderately helps. Any trojan can go and get its nice paypal.com domain with Cyrillic a's and then you are equally as screwed... This gets into the rat race issue very, very quickly...

   The setup.nsi issue is an artifact of how old this stuff really is. Its been around forever, and so many people have seen it/looked over it/written it that I completely disagree with the assumption that it is untested. It was written by luke, rewritten by me, commented on by laanjw, gmaxwell, etc, etc. And has been in 0.6 since rc1.

9. 
   laanwj commented at 6:51 PM on March 26, 2012: member

   Right, the MITM issue is completely separate, and isn't worse with embedded URLs than with embedded addresses in pages or mails. The only way to protect against that would be to sign the URLs, but that brings the the whole web-of-trust/chain-of-trust shebang into bitcoin and should be considered carefully...

10. 
    TheBlueMatt commented at 6:57 PM on March 26, 2012: member

    If everyone thinks this needs disabled for 0.6, thats fine, though Im really tired of this getting kicked down the road, it was ready to go for 0.4...

11. 
    sipa commented at 7:02 PM on March 26, 2012: member

    There are certainly risks involved when using URI's to transfer information, but this does not "worsen" the situation if even a copy-pasted static base58 address can get intercepted and replaced by a trojan. I hope we can enable this feature quickly. I also don't mind using very boost-specific patches to workaround a known bug on windows platforms.

    That said, I don't mind delaying it to 0.6.1 or very soon for 0.7.0 if nobody even noticed it didn't work since 0.6.0rc1, when the dirty hacks can be sufficiently tested.

12. gavinandresen referenced this in commit c289d95d6b on Mar 26, 2012
13. gavinandresen merged this on Mar 26, 2012
14. gavinandresen closed this on Mar 26, 2012

    ---

15. coblee referenced this in commit 23d45707bf on Jul 17, 2012
16. ptschip referenced this in commit ea55a9dbb5 on Mar 6, 2018
17. lateminer referenced this in commit 4f3dd5ef66 on Oct 30, 2019
18. DrahtBot locked this on Sep 8, 2021

Milestone
0.6.0