https://solodit.xyz/issues/g-05-rearrange-code-to-fail-early-code4rena-caviar-caviar-private-pools-git

Create a backlog of features to implement, spanning across gas efficiencies, vulnerabilities, and QA items.

• Use C4 Reports
• Use Solodit for aggregated findings across all platforms.
• Any other means

Optimizations

• #36
• #28
• #34
• #35
• #29
• #32
• #33
• #30
• #31
• #37
• #38
• #39
• #40
• #41
• #42
• #43
• #44
• #45
• #46
• #48
• #47
• #49

QA

• #50
• #51
• #52

Vulnerability

• #54
• #53
• #55
• #56

When there is a transferFrom in a contract we can infer the contract has token approval in some capacity. We should have specific analysis over a contract with approval to check for low hanging vulnerabilities that might arise due to this.

A couple examples I can think of off the top of my head. There should never be any exposed swap callbacks without authentication checks on the caller. Another one would be you should never execute arbitrary calls through an unauthenticated external function. Any admin function might be vulnerable for rugging if the right constituents are met. Probably more, we should brainstorm.

We need to decide on how to best present the report. We could take an approach that is similar to Solstat, where it generates an .md file with a description, an example and if applicable, gas savings for each item, then listing all of the line numbers that this pertains to.

We could consider injecting snippets of the codebase into the report, but we should also be mindful that the report should be clearly communicated, specific to the report and not too lengthy.

I have also seen some people generate PDF files for the report.

We should figure out the optimal way to present the report.

For Reference:

• https://docs.code4rena.com/awarding/judging-criteria
• https://docs.code4rena.com/awarding/judging-criteria/severity-categorization
• code-423n4/org#34

https://solodit.xyz/issues/3s-lenft-n12-throughout-code-variables-should-be-immutable-threesigma-none-lenft-markdown_

https://solodit.xyz/issues/3s-lenft-n01-protocoltradingswaproutersol-nonreentrant-modifier-isnt-necessary-threesigma-none-lenft-markdown_

https://solodit.xyz/issues/3s-lenft-n10-throughout-codebase-overuse-of-libraries-threesigma-none-lenft-markdown_

We should think through what we will want to use the graph for and how it will need to be constructed.

One example is reentrancy, where we would need to know all the external functions and map a path from each function to see if it is able to be reentered. Further, we need to account for any modifiers or protected calls (ex. OnlyAdmin).

We should also account for read only reentrancy.

Another example is unprotected calls. We need to know all of the external functions in a contract, as well as which functions mutate a variable that is important to the protocol (either the contract itself or other contracts that rely on the contract). For example, this could be a variable that another contract reads as a fee, or a beneficiary address, etc.

https://solodit.xyz/issues/g-01-loops-in-approvalvotingmodulepropose-can-be-consolidated-zachobront-none-optimismgovernormd-markdown

https://solodit.xyz/issues/g-02-use-calldata-instead-of-memory-for-function-arguments-that-do-not-get-mutated-code4rena-caviar-caviar-private-pools-git

https://solodit.xyz/issues/3s-lenft-n02-librariestypesdatatypessol-structs-should-be-packed-threesigma-none-lenft-markdown_

https://solodit.xyz/issues/remove-experimental-abiencoderv2-pragma-cyfrin-beanstalk-wells-markdown_

https://solodit.xyz/issues/g-01-remove-code4rena-muteio-mute-switch-versus-contest-git

The following is a list of the foundational todos to get sstan in a position where we can be seamlessly adding new features.

• #6
• #4
• #3
• #8
• #1

Arithmetic expressions within an unchecked block need to enforce that the size of the return data does not exceed the capacity of the return type. Specifically applicable for operations increasing the capacity of the data i.e. + * ^.

Bad:

function add64x64(uint128 x, uint128 y) internal pure returns (uint128) {
        unchecked {
            uint256 answer = uint256(x) + y;
            return uint128(answer);
        }
    }

Good:

function add64x64(uint128 x, uint128 y) internal pure returns (uint128) {
        unchecked {
            uint256 answer = uint256(x) + y;
            require(answer <= MAX_64x64);
            return uint128(answer);
        }
    }

Many projects use a chainlink or other oracle for on chain gas price, and token prices. For the chainlink oracle interface a call to the oracle will have call to the interface latestRoundData().

Following a call to latestRoundData() there should always be requirements to make sure the data is not stale. For Example:
Bad:

( roundId, rawPrice, , updateTime, answeredInRound ) = priceFeed.latestRoundData();

//--continue logic

Good:

( roundId, rawPrice, , updateTime, answeredInRound ) = priceFeed.latestRoundData();
require(rawPrice > 0, "Chainlink price <= 0");
require(updateTime != 0, "Incomplete round");
require(answeredInRound >= roundId, "Stale price");

I'm sure there are probably other oracles providing realtime data other than chainlink, so any other popular oracle interfaces should be included when parsing for the vulnerability.

https://solodit.xyz/issues/g-03-state-variables-can-be-cached-instead-of-re-reading-them-from-storage-code4rena-caviar-caviar-private-pools-git

https://solodit.xyz/issues/3s-lenft-n23-tradingswaproutersol-replace-line-to-reduce-gas-an-improve-readability-threesigma-none-lenft-markdown_

PragmaExtractor
ImportExtractor
VariableExtractor
StorageVariableExtractor
ImmutableExtractor
FunctionExtractor
StructExtractor
ConstructorExtractor
EventExtractor
ErrorExtractor

We should document all of the changes and advanced features that we want sstan to have so that we can update our architecture accordingly, leaving the wires out for when we are ready to integrate these features.

One feature that comes to mind immediately is reading in the entire project as a whole instead of parsing and analyzing each contract one by one. This way, we can create a graph representing the contract relationships which will allow us to conduct much more complicated analysis.

Another could be some approach to fuzzing or mutation testing if not already implemented in the test suite. For example, if the test suite is in foundry, and the tests are not robust, we could also inject code to fuzz the tests where applicable and see if the protocol breaks anywhere.

Lets talk through all of the features that we want sstan to have, and coordinate from there.

Checklist

• #10
• #8
• #6

Decide on whether to build SSTAN on top of Solstat, Pyrometer or some new approach from scratch.

Inject fuzzing, forge coverage, do static analysis, mutation testing on test suite to give coverage and test quality.