code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-periphery.sol#L531 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-periphery.sol#L543 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-periphery.sol#L520 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-periphery.sol#L517

Vulnerability details

Impact

As Canto currently lacks advanced blockchain explorer capabilities with reliable code verification (incl. libraries) an advanced, well funded adversary could create, promote and persuade to include a malicious token with modifiable decimals state variable. Modification of decimals state variable will lead to a error in oracle price computation, user position liquidation and asset pool draining if the malicious asset is not a collateral. If the malicious asset is enabled as a collateral, depending on the amounts of assets supplied, a significant to total draining of user funds pools may be possible.

Proof of Concept

Add the following function to the WETH.sol smart contract:

   function setDecimals(uint8 decimals_) public {
        _decimals = decimals_;
    }

Add the following lines to the Deployer swaps 10 times to cement observations in the pair oracle test

        await (await weth.setDecimals(32)).wait()
        await (await weth.setDecimals(1)).wait()
        let actualPrice = (await router.getUnderlyingPrice(cCanto.address)).toBigInt()
        // sample does not factor most recent observation into account
        let expected = avg(pricesCanto, 1) // observations lag behind
        console.log("actualPrice: ", actualPrice) 
        console.log("expected price: ", expected)
        // expect less than 0.1% difference in price (actual Price is TWAP) expected calculation does not weight by time
        expect(diff(actualPrice, expected)  == BigInt(0)).to.be.true

Tools Used

vscode

Recommended Mitigation Steps

Use BaseV1Pair

    uint internal immutable decimals0;
    uint internal immutable decimals1;

instead.

nivasan1 commented 2 years ago

Notice, that attack of this scale would require the user to over-ride quorum in the network, as this token would need to be supported by the Comptroller. In this case, the user would need to co-ordinate an attack amongst majority stake-holders in the network, or control a majority stake in the network.

0xean commented 2 years ago

I am going to downgrade to QA. This has so many external requirements to become feasible that it's very hard to award it as medium severity.

code-423n4 / 2022-09-canto-findings