search

code-423n4 / 2022-06-putty-findings

5 stars 0 forks source link

Maker could avoid premium on longs and strike payment on short puts by creating an order with a fake baseAsset token #406

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/1ddbec4a5242e0160da832cb46b2b3cdbb49a8af/contracts/src/PuttyV2.sol#L324 https://github.com/code-423n4/2022-06-putty/blob/1ddbec4a5242e0160da832cb46b2b3cdbb49a8af/contracts/src/PuttyV2.sol#L344

Vulnerability details

Impact

Avoid premium payment on long call or long put. Avoid strike payment on short put.

Proof of Concept

NOTE: this would not be possible if the order was created using the official DApp which has a drop down selector that restricts the base asset ("base token" in the dapp) to the official DAI or WETH tokens.

However for users interacting with the protocol by some other means - a different dapp or an API this would be possible if the maker could convince the taker they are using DAI for example, but actually provide some other fake DAI token as Order.baseAsset.

Premium is transferred from maker to taker for longs here: https://github.com/code-423n4/2022-06-putty/blob/1ddbec4a5242e0160da832cb46b2b3cdbb49a8af/contracts/src/PuttyV2.sol#L324 Strike is transferred from maker to contract for short puts here: https://github.com/code-423n4/2022-06-putty/blob/1ddbec4a5242e0160da832cb46b2b3cdbb49a8af/contracts/src/PuttyV2.sol#L344

However no validation is done on the baseAsset except to check that it is an address with bytecode: require(order.baseAsset.code.length > 0, "baseAsset is not contract");.

Tools Used

Manual analysis.

Recommended Mitigation Steps

Maintain a Whitelist of allowed baseAssets in the PuttyV2 contract.

Replace the check order.baseAsset.code.length > 0 with something like baseAssetWhitelist[order.baseAsset] == true.

This would mitigate this issue by ensuring the baseAsset is a known and verified token.

The initial whitelist would contain the addresses for the tokens listed in the DApp UI - DAI and WETH.

GalloDaSballo commented 2 years ago

Orders can be created with any token, why would the counterparty agree to be paid in a fake token?

outdoteth commented 2 years ago

Duplicate: Setting malicious or invalid erc721Assets, erc20Assets or floorTokens prevents the option from being exercised: https://github.com/code-423n4/2022-06-putty-findings/issues/50