Several contracts from root package use floating pragma ^0.8.13 and store value using assembly

Summary

Several contracts from root package use floating pragma ^0.8.13 and store value using assembly. There is a bug in solidity up to 0.8.15, when you store smth and don't read it in same slot, then it will not be stored. Because perennial attracts developers to use their code it's possible that someone will use version that is below 0.8.15 and will face the bug.

Vulnerability Detail

Perennial protocol attracts developers to built on their system. Because of that it's possible that developers will reuse the code inside perennial repo for their needs. Inside root there are a lot of different base contracts(libraries) that are used through the whole code base of another packages. These are Token18, Fixed18 for example. Such libraries use assembly to store the value. https://github.com/sherlock-audit/2023-05-perennial/blob/main/root/contracts/number/types/Fixed18.sol#L373-L377

    function store(Fixed18Storage self, Fixed18 value) internal {
        assembly ("memory-safe") {
            sstore(self, value)
        }
    }

And also they use floating solidity ^0.8.13. There is a bug up to 0.8.15 version of solidity when storing value using assembly.

Because of this floating solidity that means that developers will be able to use this library with version below 0.8.15 and as result they will face that error.

Impact

Problems with integration.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/root/contracts/token/types/Token18.sol#L155-L159 https://github.com/sherlock-audit/2023-05-perennial/blob/main/root/contracts/number/types/Fixed18.sol#L373-L377

Tool used

Manual Review

Recommendation

I recommend you to update solidity version for the libraries in the root, to avoid problems for developers.

KenzoAgada commented 1 year ago

No actual impact or POC is shown. Considering as a valid low.

arjun-io commented 1 year ago

We will consider updating the floating pragmas for library contracts to a non-buggy Solidity version. The buggy solidity version was also pointed out in the Veridise audit but they determined there was no impact of the bug on current code.