In the StableSwapFactory contract, the initialize function does not have proper protection, which allows it to be recalled by anyone. This lack of access control presents a critical security risk, as anyone can become the admin and modify important contract variables like the addresses of the LPFactory, SwapTwoPoolDeployer, and SwapThreePoolDeployer. This can lead to unauthorized control of the AMM (Automated Market Maker) system and manipulation of its core components.
Attack Scenario
An attacker could call the initialize function after the contract has been deployed and overwrite the critical variables such as the admin address. By doing so, they can become the new admin of the system and override existing token pairs with a malicious LP Token. Because the StableSwapRouter uses the getStableInfo of the StableSwapFactory, the users will swap on malicious liquidity pool.
This is an example but at the end the attacker can break everything.
Attachments
Proof of Concept (PoC) File
For my tests I change the defaultNetwork value of the config file to hardhat: defaultNetwork: "hardhat".
In the Mock folder, add the MaliciousStableSwapLPFactory.sol file.
In the tests folder, add the POC.test.ts file.
Then run npx hardhat test tests/POC.test.ts
Here is the output:
Legit admin: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Malicious admin: 0x70997970C51812dc3A010C7d01b50e0d17dc79C8
Legit LP Contract for WROSE/ROSE pair: 0xEed2C4bCC26390945BF692656F6fAB4ef66250DE
Malicious LP Contract for WROSE/ROSE pair: 0x994e1179Cf5920773630cc4E272f8850F9Da820A
Revised Code File (Optional)
Instead of using an initialize function that can be called at any time, the contract should set these variables securely through the constructor, which is only called once when the contract is deployed. Since the contract is not upgradable and does not use a proxy, this approach ensures that the admin and other critical variables are set only once and cannot be overwritten.
Github username: @Coriolan-dev Twitter username: 0xc0c0_ Submission hash (on-chain): 0x67e3dbd79b70327cb73bd01409e10b410dab4a35c156f02a5bc961093aa3eae3 Severity: high
Description: Description
In the
StableSwapFactorycontract, theinitializefunction does not have proper protection, which allows it to be recalled by anyone. This lack of access control presents a critical security risk, as anyone can become the admin and modify important contract variables like the addresses of theLPFactory,SwapTwoPoolDeployer, andSwapThreePoolDeployer. This can lead to unauthorized control of the AMM (Automated Market Maker) system and manipulation of its core components.Attack Scenario
An attacker could call the
initializefunction after the contract has been deployed and overwrite the critical variables such as the admin address. By doing so, they can become the new admin of the system and override existing token pairs with a malicious LP Token. Because theStableSwapRouteruses thegetStableInfoof theStableSwapFactory, the users will swap on malicious liquidity pool.This is an example but at the end the attacker can break everything.
Attachments
For my tests I change the
defaultNetworkvalue of the config file tohardhat:defaultNetwork: "hardhat".In the Mock folder, add the
MaliciousStableSwapLPFactory.solfile.In the tests folder, add the POC.test.ts file.
Then run
npx hardhat test tests/POC.test.tsHere is the output:
Instead of using an
initializefunction that can be called at any time, the contract should set these variables securely through the constructor, which is only called once when the contract is deployed. Since the contract is not upgradable and does not use a proxy, this approach ensures that the admin and other critical variables are set only once and cannot be overwritten.Files: