Anyone can deposit to protocol tokens of other users without approve

Summary

OUT OF SCOPE, Just for team information

Passing an arbitrary from address to transferFrom can lead to loss of funds, because anyone can transfer tokens from the from address if an approval for protocol is made.

Vulnerability Detail

User_1 approve token transfer for Protocol
User_2 can deposit User_1 token instead of User_1 without any approval since the protocol is allowed to transfer tokens.

Impact

Users tokens can be deposited to protocol without their desire

Code Snippet

BondingCurveFacet::deposit call LibBondingCurve::deposit

Code of BondingCurveFacet::deposit https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/facets/BondingCurveFacet.sol#L56-L61

Code of LibBondingCurve::deposit https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibBondingCurve.sol#L133

Tool used

Cyfrin/aderyn

Proof of Concept

Put this in BondingCurveFacet.t.sol::ZeroStateBonding

    function testPOC_DepositOthersTokens(uint32 connectorWeight, uint256 baseY) public {
        uint256 collateralDeposited;
        uint256 connWeight;
        connectorWeight = uint32(bound(connWeight, 1, 1000000));
        baseY = bound(baseY, 1, 1000000);
        vm.prank(admin);
        bondingCurveFacet.setParams(connectorWeight, baseY);

        address firstAccount = address(0x1111);

        vm.startPrank(firstAccount); // prank as Account 1
        assertEq(dollarToken.allowance(secondAccount, firstAccount), 0); // Account 1 does not have allowance from Account 2

        vm.expectEmit(true, false, false, true);
        emit Deposit(secondAccount, collateralDeposited);
        bondingCurveFacet.deposit(collateralDeposited, secondAccount); // Account 1 deposit tokens of Account 2
    }

Recommendation

Change _recipient to msg.sender in LibBondingCurve::deposit

- dollar.transferFrom(_recipient, address(this), _collateralDeposited);
+ dollar.transferFrom(msg.sender, address(this), _collateralDeposited);

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

Not issue