Inline assembly bug on 0.8.13 compiler

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x4ed0129a0d3fcaf9ecdb6e83ab300fbe24511e342b6cf330e4b124a1c37f81b4 Severity: low

Description: Description\ Full issue : https://github.com/ethereum/solidity-blog/blob/499ab8abc19391be7b7b34f88953a067029a5b45/_posts/2022-06-15-inline-assembly-memory-side-effects-bug.md

SafeWebAuthnSharedSigner is using compile >= 0.8.0 But there is a optimiser side effect bug of medium severity when 0.8.13 is used for compiling an assembly code. So its better to use >= 0.8.15 here.

https://github.com/hats-finance/Safe-0x2909fdefd24a1ced675cb1444918fa766d76bdac/blob/9a18245f546bf2a8ed9bdc2b04aae44f949ec7a0/modules/passkey/contracts/4337/SafeWebAuthnSharedSigner.sol#L2

safe-modules/src/4337/SafeWebAuthnSharedSigner.sol
1: // SPDX-License-Identifier: LGPL-3.0-only
2: pragma solidity >=0.8.0;

Attack Scenario\

The Yul optimizer considers all memory writes in the outermost Yul block that are never read from as unused and removes them. This is valid when that Yul block is the entire Yul program, which is always the case for the Yul code generated by the new via-IR pipeline. Inline assembly blocks are never optimized in isolation when using that pipeline. Instead they are optimized as a part of the whole Yul input.

However, the legacy code generation pipeline (which is still the default) runs the Yul optimizer individually on an inline assembly block if the block does not refer to any variables defined in the surrounding Solidity code. Consequently, memory writes in such inline assembly blocks are removed as well, if the written memory is never read from in the same assembly block, even if the written memory is accessed later, for example by a subsequent inline assembly block.

Fortunately, the fact that the legacy code generation pipeline does not run the Yul optimizer at all on inline assembly blocks that do access Solidity variables, reduces the number of affected cases significantly. Most inline assembly blocks either read or write values from or to variables defined in the surrounding Solidity code, are entirely self-contained, or take over the program flow until the end of the transaction. Thereby, the bug is unlikely to occur in practice and its adverse effects should in most cases be easily detectable in tests. However, since the consequences in affected cases can be severe, we assigned it a severity of "medium".

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

[nlordell]

Correct me if I'm wrong but this would only affect other contracts that would use the contracts in scope? AFAIU the contracts in scope are not affected right?

[0xRizwan]

So its better to use >= 0.8.15 here.

@nlordell Contracts are being deployed with 0.8.24 so i think this issue wont produce.

[nlordell]

This is my understanding as well. Just wanted to double check with the submitter that they didn't have another angle they were proposing.

[nlordell]

I will mark this issue as invalid for now, please respond if you do not agree with the decision.