code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/EUSD.sol#L169-L185

Vulnerability details

Impact

Using the approve and increaseAllowance methods, the owner's allowance for a spender may hit type(uint256).max accidently, which will then allow the spender to have infinite allowance.

The spender can then exploit this infinite allowance without the owner being aware of it.

Proof of Concept

Here is a proof of concept test (Please note that _spendAllowance was made public to enable using this method directly):

The owner grants an allowance of 1000 to the spender
Spender spends 500 allowance
Overtime, the owner again grants an allowance of to the spender, which bumps the allowance to type(uint256).max for the spender
Now the spender has infinite allowance

describe("EUSD", function () {
    it("does not change the allowance if the spender gets assigned uint256.max allowance, making it infinite", async function () {
        const [owner, config, spender] = await ethers.getSigners();
        console.log("Config: ", config.address);
        console.log("Owner: ", owner.address);
        console.log("Spender: ", spender.address);

        console.log();

        const EUSD = await ethers.getContractFactory("EUSD");
        const eusd = await EUSD.deploy(config.address);

        await eusd.increaseAllowance(spender.address, 1000);
        console.log(`Increasing allowance`);
        console.log(`Owner ${owner.address}`);
        console.log(`Spender ${spender.address}`);
        console.log(`Allowance Increased By 1000`);
        console.log();

        const previousAllowance = await eusd.allowance(owner.address, spender.address);
        console.log(`Current allowance ${previousAllowance}`);
        console.log();

        await eusd._spendAllowance(owner.address, spender.address, 500);
        console.log(`Spending allowance`);
        console.log(`Owner ${owner.address}`);
        console.log(`Spender ${spender.address}`);
        console.log(`Amount 500`);
        console.log();

        const newAllowance = await eusd.allowance(owner.address, spender.address);
        console.log(`Current allowance ${newAllowance}`);
        console.log(`Change in allowance: ${newAllowance - previousAllowance}\n`);
        console.log();

        await eusd.increaseAllowance(spender.address, ethers.constants.MaxUint256.sub(newAllowance));

        console.log(`Increasing allowance`);
        console.log(`Owner ${owner.address}`);
        console.log(`Spender ${spender.address}`);
        console.log(`Allowance Increased By ${ethers.constants.MaxUint256.sub(newAllowance)}`);
        console.log();

        const newNewAllowance = await eusd.allowance(owner.address, spender.address);
        console.log(`Current allowance ${newNewAllowance}`);
        console.log(`Change in allowance: ${(ethers.BigNumber.from(newNewAllowance).sub(ethers.BigNumber.from(newAllowance)))}\n`);
        console.log();

        await eusd._spendAllowance(owner.address, spender.address, 500);

        console.log(`Spending allowance`);
        console.log(`Owner ${owner.address}`);
        console.log(`Spender ${spender.address}`);
        console.log(`Amount 500`);
        console.log();

        const finalAllowance = await eusd.allowance(owner.address, spender.address);
        console.log(`Current allowance ${finalAllowance}`);
        console.log(`Change in allowance: ${finalAllowance - newNewAllowance}\n`);
        console.log();
    });
});

Output:

Config:  0x70997970C51812dc3A010C7d01b50e0d17dc79C8
Owner:  0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Spender:  0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC

Increasing allowance
Owner 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Spender 0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC
Allowance Increased By 1000

Current allowance 1000

Spending allowance
Owner 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Spender 0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC
Amount 500

Current allowance 500
Change in allowance: -500

Increasing allowance
Owner 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Spender 0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC
Allowance Increased By 115792089237316195423570985008687907853269984665640564039457584007913129639435

Current allowance 115792089237316195423570985008687907853269984665640564039457584007913129639935
Change in allowance: 115792089237316195423570985008687907853269984665640564039457584007913129639435

Spending allowance
Owner 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Spender 0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC
Amount 500

Current allowance 115792089237316195423570985008687907853269984665640564039457584007913129639935
Change in allowance: 0

Tools Used

VSCodium, and manual analysis

Recommended Mitigation Steps

Do not rely on type(uint256).max for determining if a spender has infinite allowance. Rather create a new map like mapping(address => mapping(address => bool)) private hasInfiniteAllowances; to keep track of who has granted infinite allowance.