60,000 USDC
Submission Details
Severity:
Malicious state update after transfer
Summary
In `Escrow.sol`, the state update to `set _escrow[collection][id] = address(0x0)` happens after an attempt on transfer.
Vulnerability Details
The malicious state update could be problematic since if the transfer fails for some reason (e.g., the safeTransferFrom function reverts or throws an exception), `_escrow` mapping would not be updated.
```javascript
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id);
} else {
// TODO:
// Check here if the token supply is currently 0.
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
@> _escrow[collection][id] = address(0x0);
```
Impact
The consequence of this error could cheat the system that the asset is escrowed even though the transfer did not actually occur. SUch can cause inconsistencies and potential exploits.
Tools Used
Manual Review
Recommendations
Ensure that the state update is happening before transfer happens. Therefore, consider moving the line `_escrow[collection][id] = address(0x0)` before token transfer. Hence, even if the transfer has failed, the state of the escrow would still be updated correctly.
```javascript
@> _escrow[collection][id] = address(0x0);
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id);
} else {
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
```
Prize pool breakdown
Total prize
60,000 USDC
nSLOC
2,30126 USDC / LOC
50,000 USDC
5,500 USDC
4,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.