c4-bot-4 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-taiko/blob/main/packages/protocol/contracts/L1/tiers/MainnetTierProvider.sol#L66-L70

Vulnerability details

Description

The issue exists for both MainnetTierProvider.sol and TestnetTierProvider.sol. For this report, we shall concentrate only on describing it via MainnetTierProvider.sol.

The proving tier is chosen by the getMinTier() function which accepts a _rand param.

  File: contracts/L1/tiers/MainnetTierProvider.sol

  66:               function getMinTier(uint256 _rand) public pure override returns (uint16) {
  67:                   // 0.1% require SGX + ZKVM; all others require SGX
  68: @--->             if (_rand % 1000 == 0) return LibTiers.TIER_SGX_ZKVM;
  69:                   else return LibTiers.TIER_SGX;
  70:               }

If _rand % 1000 == 0, a costlier tier TIER_SGX_ZKVM is used instead of the cheaper TIER_SGX. The _rand param is passed in the form of meta_.difficulty which is calculated inside proposeBlock():

  File: contracts/L1/libs/LibProposing.sol

  199:                  // Following the Merge, the L1 mixHash incorporates the
  200:                  // prevrandao value from the beacon chain. Given the possibility
  201:                  // of multiple Taiko blocks being proposed within a single
  202:                  // Ethereum block, we choose to introduce a salt to this random
  203:                  // number as the L2 mixHash.
  204: @--->            meta_.difficulty = keccak256(abi.encodePacked(block.prevrandao, b.numBlocks, block.number));
  205:          
  206:                  // Use the difficulty as a random number
  207:                  meta_.minTier = ITierProvider(_resolver.resolve("tier_provider", false)).getMinTier(
  208: @--->                uint256(meta_.difficulty)
  209:                  );

As can be seen, all the parameters used in L204 to calculate meta_.difficulty can be known in advance and hence a proposer can choose not to propose when meta_.difficulty modulus 1000 equals zero, because in such cases it will cost him more to afford the proof (sgx + zk proof in this case).

Impact

Since the proposer will now wait for the next or any future block to call proposeBlock() instead of the current costlier one, transactions will now take longer to finalilze.

If _rand were truly random, it would have been an even playing field in all situations as the proposer wouldn't be able to pick & choose since he won't know in advance which tier he might get. We would then truly have:

  67:                   // 0.1% require SGX + ZKVM; all others require SGX

Tools Used

Manual review

Recommended Mitigation Steps

Consider using VRF like solutions to make _rand truly random.

Assessed type

Other

c4-pre-sort commented 8 months ago

minhquanym marked the issue as primary issue

c4-pre-sort commented 8 months ago

minhquanym marked the issue as sufficient quality report

dantaik commented 8 months ago

This is a very well known issue.

Using VRF creates a third party dependency which may be a bigger risk for a Based rollup. We'll monitor how this plays out and mitigate the issue later.

adaki2004 commented 8 months ago

Eventually we will have only 1 (1 "aggregated ZK multiproof") proof tier, which will be the default/min too. (Maybe keeping guardian for a while to be as a failsafe, but that one also cannot be "picked" with thispseudoe random calculation). Also Taiko foundation will run a proposer node, so in case noone is willing to propose to avoid fees, we will, regardless of cost - at least until we reach the 1 tier maturity.

c4-sponsor commented 8 months ago

dantaik (sponsor) acknowledged

c4-judge commented 7 months ago

0xean marked the issue as selected for report

c4-judge commented 7 months ago

0xean marked the issue as satisfactory

genesiscrew commented 7 months ago

Considering this report and the responses from the sponsors, I am unable to see how this would impact the function of the protocol in such a way that would deem it a medium risk. I personally think this is informational. The report states proving will Take longer because it assumes all proposers will want to avoid paying fees because they can predict the block difficulty. I find that a bit of a stretch.

adaki2004 commented 7 months ago

Considering this report and the responses from the sponsors, I am unable to see how this would impact the function of the protocol in such a way that would deem it a medium risk. I personally think this is informational. The report states proving will Take longer because it assumes all proposers will want to avoid paying fees because they can predict the block difficulty. I find that a bit of a stretch.

Not the proving but the liveness (proposing) would take longer as provers would deny to grant signatures to prove blocks - which's evaluation i happening during proposeBlock.

But at least +2 years post mainnet taiko foundation is commited to proposeBlock every X time intervals (even if not breaking even) to keep the liveness and get over this.

And as stated, by the time hopefully this minTier() will vanish in that time - hopefully even in months after launch (not years) when ZK is cheap enough. So for now we would say it is a known issue, we are aware of.

t0x1cC0de commented 7 months ago

Thank you for the inputs. From what I see, this is being acknowledged by the sponsor as a valid issue which is known to the team. Also important to note that it wasn't mentioned in the list of C4 "known issues" section on the audit page, so should qualify as a Medium.