Closed code423n4 closed 2 years ago
https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L344
Using transfer and transferFrom instead of their safe alternatives may result in transactions fail silently.
Using token transferFrom functions instead of safeTransferFrom (https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L344) which is highly discouraged and can cause NFTs to be stuck in the case of the transaction not reverting on failed transfers, also, in this case, because the option tokens are burnt in the process (https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L334), calling the function a second time would not mitigate the issue, since the option tokens would be gone. There’s also precedents of this vulnerability as seen here https://github.com/code-423n4/2022-01-trader-joe-findings/issues/12
Manual code review
We suggest you to check all of your contracts and fix this issue by implementing safeTransfer and safeTransferFrom instead of transfer and transferFrom where applicable.
use safeTransferFrom to prevent stuck NFTs: https://github.com/code-423n4/2022-05-cally-findings/issues/38
Lines of code
https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L344
Vulnerability details
Impact
Using transfer and transferFrom instead of their safe alternatives may result in transactions fail silently.
Proof of Concept
Using token transferFrom functions instead of safeTransferFrom (https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L344) which is highly discouraged and can cause NFTs to be stuck in the case of the transaction not reverting on failed transfers, also, in this case, because the option tokens are burnt in the process (https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L334), calling the function a second time would not mitigate the issue, since the option tokens would be gone. There’s also precedents of this vulnerability as seen here https://github.com/code-423n4/2022-01-trader-joe-findings/issues/12
Tools Used
Manual code review
Recommended Mitigation Steps
We suggest you to check all of your contracts and fix this issue by implementing safeTransfer and safeTransferFrom instead of transfer and transferFrom where applicable.