0 Members and 1 Guest are viewing this topic.
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 ?
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.
Quote from: vikram on December 14, 2014, 09:08:35 pmQuote from: arhag on December 12, 2014, 11:34:54 pm 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?
Quote from: arhag on December 12, 2014, 11:34:54 pm 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.
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?
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
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.
I suppose it could reject any transaction that has an expiration time set more than 48 hours into the future relative to the present time (time at which the delegate's client is considering the transaction for inclusion in the next block), although I don't really understand the purpose of that limitation.
But what I mean is couldn't I today create and sign (but not broadcast) a transaction that spends some of my balance and is set to expire in January 2, 2015, and then wait until January 1, 2015 to broadcast it and have it accepted into the blockchain?
There is an upper bound of 48 hours that the blockchain allows.
However, the 3 of us that put together the upgrade all still managed to completely overlook this problem due to the stress of trying to fix the malleability security issue as fast as possible. The moral of the story is--security issue or not--we need a better process for live upgrades:Quote from: bytemaster on December 12, 2014, 07:14:04 pmWe are reviewing our options, but the conclusion is clear. DevShares needs to come out as soon as possible because it is the most likely way we would have caught this bug.
We are reviewing our options, but the conclusion is clear. DevShares needs to come out as soon as possible because it is the most likely way we would have caught this bug.
Looking at the initial theft report at https://bitsharestalk.org/index.php?topic=10877.msg143174#msg143174 it seems that his private keys were compromised. This is not related to the transaction malleability issue that 0.4.25 and 0.4.26 were intended to address.
Quote from: arhag on December 14, 2014, 10:09:34 pmI can understand it is not a priority since there doesn't seem to be anyone in the community claiming they had any funds stolen or unauthorized withdrawals, but it is incredibly peculiar that there were duplicates (assuming the expiration time is always set to a fixed offset of the current time) and it would be interesting to investigate further. That's quite not true! We have at least one official case with stolen funds...( from @educatewarrior for 1million BTS!!! ) I would seriously investigate it further if it is some how related!!! And if yes, our community MUST help the victims to get their "stolen"(?) funds back ! For every problem, exist at least on solution... I am very happy we found the security flow soon enough ...Sent from my ALCATEL ONE TOUCH 997D
I can understand it is not a priority since there doesn't seem to be anyone in the community claiming they had any funds stolen or unauthorized withdrawals, but it is incredibly peculiar that there were duplicates (assuming the expiration time is always set to a fixed offset of the current time) and it would be interesting to investigate further.
That's quite not true! We have at least one official case with stolen funds...( from @educatewarrior for 1million BTS!!! ) I would seriously investigate it further if it is some how related!!! And if yes, our community MUST help the victims to get their "stolen"(?) funds back ! For every problem, exist at least on solution... I am very happy we found the security flow soon enough ...
I guess whether I am worried about it or not depends on how the expiration time is calculated. Can the expiration time be left unset in the transactions? Did the client at any point in time in BitShares history leave the expiration time blank? Does the client by default use some fixed offset after the time the transaction is constructed as the expiration time? Is the granularity of the expiration time 10 seconds or less?If the old client didn't set an expiration time, then it is totally understandable why there could be multiple duplicates (in the sense that they are the same operation) that are each legitimate. But if the client always sets an expiration time and uses a fixed offset from the current time as the expiration time, I find the probability of creating a transaction with the same operations and same expiration time to be incredibly unlikely. I can understand it is not a priority since there doesn't seem to be anyone in the community claiming they had any funds stolen or unauthorized withdrawals, but it is incredibly peculiar that there were duplicates (assuming the expiration time is always set to a fixed offset of the current time) and it would be interesting to investigate further.
However, the 3 of us that put together the upgrade all still managed to completely overlook this problem due to the stress of trying to fix the malleability security issue as fast as possible. The moral of the story is--security issue or not--we need a better process for live upgrades:
Quote from: bytemaster on December 12, 2014, 07:14:04 pmWe applied the change to the main network and discovered that there was one transaction that was considered a duplicate under the new system that was processed by the old system. Can you provide anymore details about that. Was that a "legitimate" duplicate in the sense that it followed identical operations as a previous transaction sent to the blockchain but was signed with different signatures? 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? If it wasn't "legitimate" was it a transaction that uses a non-canonical signature or alternatively stuffed a superfluous signature to create a unique transaction ID? If so, does that mean that was an intentional attack transaction?
We applied the change to the main network and discovered that there was one transaction that was considered a duplicate under the new system that was processed by the old system.
Quote from: bytemaster on December 12, 2014, 07:14:04 pmDelegates started upgrading. Duplicate transaction checking was "disabled" until the specified fork block. Delegates started producing blocks that included duplicate transactions and thus were rejected by the non upgraded clients. So v0.4.25 wasn't using the old duplicate transaction checking code until block 1249400 when it would then switch to the new one? It was simply not checking for duplicate transactions AT ALL until block 1249400? And then when people submitted a transaction it appeared once to a delegate client but then lingered around and the next delegate client included the same transaction again in their block because they didn't have the code to check it was a duplicate? Am I understanding that correctly? I think I am missing something.
Delegates started upgrading. Duplicate transaction checking was "disabled" until the specified fork block. Delegates started producing blocks that included duplicate transactions and thus were rejected by the non upgraded clients.
The reason is bad software engineering practices. Lack of solid test framework. It's irresponsible and should be fixed yesterday.
Can I safely assume that 0.4.26 will stay around for some days to weeks now? I will be leaving to airport for a 15h flight ..
We implemented a change in how we check for duplicate transactions. Tested it. It worked.We applied the change to the main network and discovered that there was one transaction that was considered a duplicate under the new system that was processed by the old system. Vikram disabled duplicate checking until the scheduled fork block so that we could validate the old chain.Vikram tested syncing up with the old chain and it worked. Vikram tested producing a block with the upgraded version and it worked. Announce upgrade.Delegates started upgrading. Duplicate transaction checking was "disabled" until the specified fork block. Delegates started producing blocks that included duplicate transactions and thus were rejected by the non upgraded clients. So the testing we needed was "dev share" test net because the bug was is the "handoff / upgrade" code that didn't reveal itself until after we had a chain with multiple delegates running the new code. In retrospect the issue could have been easily avoided with a little thinking, but we were rushing to get a security fix in.We are reviewing our options, but the conclusion is clear. DevShares needs to come out as soon as possible because it is the most likely way we would have caught this bug.
Quote from: Empirical1.1 on December 12, 2014, 12:16:16 pmAt least the VOTE presentation went well...yeh, but these kinds of mistakes can reallllly shake confidence in the whole system.
At least the VOTE presentation went well...