code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-basin/blob/f15fe66d57c2f226c232685d16f297e54bcc0939/src/libraries/LibWellConstructor.sol#L71-L81

Vulnerability details

Impact

Writing this

string memory name = LibContractInfo.getSymbol(address(_tokens[0]));
string memory symbol = name;

does not copy the content of name in symbol, what it does is copy the POINTER to the place where the name is stored. Thus, one change in one of them leads to the other pointing to the "updated" variable, breaking the working process of the function and passing wrong arguments to the init function (so the high severity`

Proof of Concept

From Jean Cvllr awesome tutorials about data locations we have

In reality, what happened under the hood is that we create two pointers to memory, named by the variables data and greetings.

When we do data = greetings , we think we are assigning the value cafecafe to the variable data. But we are not assigning anything at all here! We are giving the following instruction to the EVM:

“variable data, I order you to point to the same location in memory that the variable greetings point to!”

(you can read the Yellow paper too but the above is more user-friendly). That means the next code does the same to the two variables because both point to the same place, so one change will affect the other the same way

for (uint256 i = 1; i < _tokens.length; ++i) {
        name = string.concat(name, ":", LibContractInfo.getSymbol(address(_tokens[i])));
        symbol = string.concat(symbol, LibContractInfo.getSymbol(address(_tokens[i])));
}
name = string.concat(name, " ", LibContractInfo.getName(_wellFunction.target), " Well");
symbol = string.concat(symbol, LibContractInfo.getSymbol(_wellFunction.target), "w");

Because of that, the initFunctionCall = abi.encodeWithSignature("init(string,string)", name, symbol); is broken given the fact that name and symbol are the same.

Tools Used

Manual analysis

Recommended Mitigation Steps

Use local variables instead of memory references/pointers

Assessed type

Error

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

141345 commented 1 year ago

need to recheck the POC validity

maybe QA is more appropriate

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #199

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)