You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The reason that asserts should never be triggered in bug-free code is that triggering an assert() causes the transaction to use up more space on the blockchain than is necessary, since the transaction uses all gas provided by the user in the transaction's gasLimit, whereas triggering a require() only uses up the gas used up to the point of the failed require() statement. Using up more gas than necessary hurts the public good of blockspace, by unnecessarily filling up the block with more gas than is strictly necessary for the failed transaction to use. It is useful in development environments to distinguish between allowable reverts and unallowable reverts in automated testing, but asserts should not be reached in live code.
Proof of Concept
Alice can call RCFactory.createMarket() with a value greater than two for _mode // [2]
RCFactory.createMarket() will then call RCMarket.initialize() // [3]
RCMarket.initialize() will trigger an assert, which should never be reachable by a user, even with invalid external input. // [4]
Handle
jvaqa
Vulnerability details
RCMarket.initialize() improperly uses assert() rather than require()
Impact
According to Solidity documentation for v0.8.0:
"Properly functioning code should never [trigger an assert], not even on invalid external input. If this happens, then there is a bug in your contract which you should fix."[1]
[1] https://docs.soliditylang.org/en/v0.8.0/control-structures.html#error-handling-assert-require-revert-and-exceptions
The reason that asserts should never be triggered in bug-free code is that triggering an assert() causes the transaction to use up more space on the blockchain than is necessary, since the transaction uses all gas provided by the user in the transaction's gasLimit, whereas triggering a require() only uses up the gas used up to the point of the failed require() statement. Using up more gas than necessary hurts the public good of blockspace, by unnecessarily filling up the block with more gas than is strictly necessary for the failed transaction to use. It is useful in development environments to distinguish between allowable reverts and unallowable reverts in automated testing, but asserts should not be reached in live code.
Proof of Concept
Alice can call RCFactory.createMarket() with a value greater than two for _mode // [2]
RCFactory.createMarket() will then call RCMarket.initialize() // [3]
RCMarket.initialize() will trigger an assert, which should never be reachable by a user, even with invalid external input. // [4]
Recommended Mitigation Steps
Change this:
assert(_mode <= 2);
To this:
require(_mode <= 2);
[1] https://docs.soliditylang.org/en/v0.8.0/control-structures.html#error-handling-assert-require-revert-and-exceptions
[2] https://github.com/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/RCFactory.sol#L466
[3] https://github.com/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/RCFactory.sol#L582
[4] https://github.com/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/RCMarket.sol#L202
The text was updated successfully, but these errors were encountered: