search

code-423n4 / 2024-07-optimism-findings

3 stars 0 forks source link

For instructions such as DIV and mthi, store register will be set to 0, which is unexpectedly. #81

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-07-optimism/blob/main/packages/contracts-bedrock/src/cannon/MIPS.sol#L383-L445 https://github.com/code-423n4/2024-07-optimism/blob/main/packages/contracts-bedrock/src/cannon/MIPS.sol#L804

Vulnerability details

Impact

Store register will be mistakenly set to 0.

Details

In the function handleHiLo, when _storeReg is not 0, the store reg will be set to val. From the perspective of MISP instruction design, the store register needs to be set only when _func is mfhi and mflo. However, in the implementation, for the instructions mthi, mtlo, mult, div, etc., store register may also be overwritten. The same problem occurs at the end of the function step.

Tools Used

Vscode

Recommended Mitigation Steps

Place store register assignments in each instruction branch rather than at the end of a function. For exapmle:

// mfhi: Move the contents of the HI register into the destination
if (_func == 0x10 && _storeReg != 0) {
    state.registers[_storeReg] = state.hi;
}

Assessed type

Other

refcell commented 4 months ago

The val variable that is written to the register at _storeReg is only set in the mfhi and mflo function branches so this is incorrect.

c4-judge commented 4 months ago

zobront marked the issue as unsatisfactory: Invalid