Author Topic: What is the Reason For 0.4.25 Forks ?  (Read 2829 times)

0 Members and 1 Guest are viewing this topic.

Offline xeroc

  • Board Moderator
  • Hero Member
  • *****
  • Posts: 12911
  • ChainSquad GmbH
    • View Profile
    • ChainSquad GmbH
  • BitShares: xeroc
  • GitHub: xeroc
Re: What is the Reason For 0.4.25 Forks ?
« Reply #30 on: December 15, 2014, 01:34:38 am »
So .. no it's about time to have a public mail address for security issues to report .. that mail address HAS to be publicly accessible ..
put it
 - in the footer of bitshares.org
 - the wiki
 - the subreddits
 - the client

Also please offer a bounty!
Give BitShares a try! Use the http://testnet.bitshares.eu provided by http://bitshares.eu powered by ChainSquad GmbH

Offline arhag

  • Hero Member
  • *****
  • Posts: 1214
    • View Profile
    • My posts on Steem
  • BitShares: arhag
  • GitHub: arhag
Re: What is the Reason For 0.4.25 Forks ?
« Reply #31 on: December 15, 2014, 02:01:04 am »
This fix was rushed out because once the existence of the security flaw was publicly disclosed, we felt under extreme time pressure because any technically competent attacker reading our public discussions would have been able to use the unpatched flaw to do serious damage.

We need to do a better job of telling all internal developers and community contributors not to discuss security vulnerabilities over public channels until a fix is deployed.  A little more caution along these lines would quite possibly have resulted in a patch developed over a longer time frame, with more thorough testing, under less pressure.

Yeah, things could have gone better in how this was handled. Some suggestions:
  • Once bytemaster realized there was a vulnerability thanks to ElMato's question submitted to the forum on Wednesday, December 10, he probably shouldn't have responded to that thread with a post containing content identical to the OP in https://github.com/BitShares/bitshares/issues/1129. Just speaking for myself, I didn't realize there was a vulnerability when reading ElMato's question, but when I read bytemaster's post I immediately realized the extent of this vulnerability (and I am sure so would many other technical people who happened to read that thread). Thankfully, the situation was corrected quickly by deleting the thread, but it would have been better if bytemaster never made that post, or revealed it on GitHub either, until the fix was completed, well tested, and ready to be pushed to the delegates.

  • GitHub revealed too much information throughout this whole process. Although the forum thread was short lived, bytemaster's post (https://github.com/BitShares/bitshares/issues/1129) to GitHub on Thursday 2:44am EST revealed it again to the public and this time stayed up for all to see. Although that post was accompanied with a fix (https://github.com/BitShares/bitshares/commit/d0c6fa174ee88b706e6c08a102cde5bddfe4212d) to the canonical signature issue, there were still two problems with this. First, the canonical signature issue wasn't an actual fix for the bigger issue of transaction malleability, since duplicate transactions with unique IDs could still be created using superfluous signatures. Further discussion with the dev team and spending some time thinking it through would have revealed this because eventually they came to that conclusion and fixed the problem properly with this fix (https://github.com/BitShares/bitshares/commit/ad4c5d851b90843d1a20d8ca9ab410baf5aafb1b) 9 hours later (well the upgrade code was messed up still, but at least the validation code for after block 1249400 actually solved the transaction malleability vulnerability properly). Second, even with these commits, the code was still not done and certainly not properly tested. My point with all of this is that the reveal of the vulnerability on GitHub through the issue submission and these commits was premature for a security vulnerability. I don't think these things should be revealed publicly on GitHub until a proper, well-tested fix is ready to go for the delegates to download. Discussion and development for a fix to a critical security issue should be done in private IMO. Does GitHub provide a private mode so that the internal devs can still use these useful tools without leaking information to the public prematurely? They can always reveal the issues/discussion/commits to the public later on after the well-tested upgrade that fixes the issue is ready to go for the delegates.

I don't intend to point fingers or assign blame with this post. I just would like us to figure out the proper procedures to follow for the future in case another security vulnerability is discovered. We should of course also discuss what the proper standards for testing should be for fixes to important security vulnerabilities (and the differences in standards for different classes of vulnerabilities, depending on severity). A balance must be struck with the testing standards between getting functional-enough code out that fixes the issue (and doesn't have bugs that make the issue worse) and minimizing the time that the vulnerable version of the software is left running in the open.

Offline liondani

  • Hero Member
  • *****
  • Posts: 3733
  • Inch by inch, play by play
    • View Profile
    • My detailed info
  • BitShares: liondani
  • GitHub: liondani
Re: What is the Reason For 0.4.25 Forks ?
« Reply #32 on: December 15, 2014, 03:36:46 am »


Quote from: vikram link=topic=12208.msg162170#msg162170

We have not done a thorough analysis or inventory, but there were a number of transactions (not many--definitely less than 200 in the entire blockchain) that were duplicates under the new rules yet valid under the old rules. That is to say, they had exactly the same operations and expiration time but different signatures as you said. Almost all of such transactions were quite old; there was at least one extremely recent one but it was very small and did not look like an attack. It is difficult to say whether anything else appeared as an attack without doing a full review of the offending transactions. It is not a priority, but I can take a closer look if you think that is important.

can you post the duplicate transactions here so anybody can review them ?



Sent from my ALCATEL ONE TOUCH 997D


Offline vikram

Re: What is the Reason For 0.4.25 Forks ?
« Reply #33 on: December 15, 2014, 03:42:54 am »


Quote from: vikram link=topic=12208.msg162170#msg162170

We have not done a thorough analysis or inventory, but there were a number of transactions (not many--definitely less than 200 in the entire blockchain) that were duplicates under the new rules yet valid under the old rules. That is to say, they had exactly the same operations and expiration time but different signatures as you said. Almost all of such transactions were quite old; there was at least one extremely recent one but it was very small and did not look like an attack. It is difficult to say whether anything else appeared as an attack without doing a full review of the offending transactions. It is not a priority, but I can take a closer look if you think that is important.

can you post the duplicate transactions here so anybody can review them ?



Sent from my ALCATEL ONE TOUCH 997D

Sure, I can compile a list as part of: https://github.com/BitShares/bitshares/issues/1142

merockstar

  • Guest
Re: What is the Reason For 0.4.25 Forks ?
« Reply #34 on: December 15, 2014, 04:20:52 pm »
I remember a thread a few months back where a guy created an account to come say he had accidentally made bitshares out of thin air, and the community kind of received it with skepticism.

trying to find the thread now but it's elusive.

I wonder if the one duplicate was that guy.

Offline ElMato

  • Sr. Member
  • ****
  • Posts: 288
    • View Profile
Re: What is the Reason For 0.4.25 Forks ?
« Reply #35 on: December 16, 2014, 04:58:37 am »
If so, does the new code allow for such possibilities in the future (using some nonce or something) or does the new client somehow construct the transactions to not ever be identical?

In practice, nothing really changes for the end-user because subsequent identical transactions they construct will likely have different expiration dates. I do not know how those previously mentioned transactions with identical operations and expiration dates were created. There is also an additional reserved field in transactions that is currently not used, but could include a nonce if this ever becomes a problem in the future.

@Virkram, can you point me to one pair of those transaction?
They were TITAN transactions or simple ones?




Offline vikram

Re: What is the Reason For 0.4.25 Forks ?
« Reply #36 on: January 07, 2015, 12:13:19 am »


Quote from: vikram link=topic=12208.msg162170#msg162170

We have not done a thorough analysis or inventory, but there were a number of transactions (not many--definitely less than 200 in the entire blockchain) that were duplicates under the new rules yet valid under the old rules. That is to say, they had exactly the same operations and expiration time but different signatures as you said. Almost all of such transactions were quite old; there was at least one extremely recent one but it was very small and did not look like an attack. It is difficult to say whether anything else appeared as an attack without doing a full review of the offending transactions. It is not a priority, but I can take a closer look if you think that is important.

can you post the duplicate transactions here so anybody can review them ?

If so, does the new code allow for such possibilities in the future (using some nonce or something) or does the new client somehow construct the transactions to not ever be identical?

In practice, nothing really changes for the end-user because subsequent identical transactions they construct will likely have different expiration dates. I do not know how those previously mentioned transactions with identical operations and expiration dates were created. There is also an additional reserved field in transactions that is currently not used, but could include a nonce if this ever becomes a problem in the future.

@Virkram, can you point me to one pair of those transaction?
They were TITAN transactions or simple ones?

I've posted a list of the duplicates here: https://github.com/BitShares/bitshares/issues/1142#issuecomment-68958340