Skip to content

Commit

Permalink
Avoid returnbomb in ERC165Checker (#3587)
Browse files Browse the repository at this point in the history
Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
(cherry picked from commit dc4869e)
  • Loading branch information
Amxx authored and frangio committed Jul 27, 2022
1 parent 67b2572 commit 8fb5f57
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 3 deletions.
18 changes: 18 additions & 0 deletions contracts/mocks/ERC165/ERC165ReturnBomb.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../../utils/introspection/IERC165.sol";

contract ERC165ReturnBombMock is IERC165 {
function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
if (interfaceId == type(IERC165).interfaceId) {
assembly {
mstore(0, 1)
}
}
assembly {
return(0, 101500)
}
}
}
16 changes: 13 additions & 3 deletions contracts/utils/introspection/ERC165Checker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,19 @@ library ERC165Checker {
* Interface identification is specified in ERC-165.
*/
function _supportsERC165Interface(address account, bytes4 interfaceId) private view returns (bool) {
// prepare call
bytes memory encodedParams = abi.encodeWithSelector(IERC165.supportsInterface.selector, interfaceId);
(bool success, bytes memory result) = account.staticcall{gas: 30000}(encodedParams);
if (result.length < 32) return false;
return success && abi.decode(result, (uint256)) > 0;

// perform static call
bool success;
uint256 returnSize;
uint256 returnValue;
assembly {
success := staticcall(30000, account, add(encodedParams, 0x20), mload(encodedParams), 0x00, 0x20)
returnSize := returndatasize()
returnValue := mload(0x00)
}

return success && returnSize >= 0x20 && returnValue > 0;
}
}
20 changes: 20 additions & 0 deletions test/utils/introspection/ERC165Checker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const ERC165MissingData = artifacts.require('ERC165MissingData');
const ERC165MaliciousData = artifacts.require('ERC165MaliciousData');
const ERC165NotSupported = artifacts.require('ERC165NotSupported');
const ERC165InterfacesSupported = artifacts.require('ERC165InterfacesSupported');
const ERC165ReturnBombMock = artifacts.require('ERC165ReturnBombMock');

const DUMMY_ID = '0xdeadbeef';
const DUMMY_ID_2 = '0xcafebabe';
Expand Down Expand Up @@ -243,4 +244,23 @@ contract('ERC165Checker', function (accounts) {
expect(supported[0]).to.equal(false);
});
});

it('Return bomb resistance', async function () {
this.target = await ERC165ReturnBombMock.new();

const tx1 = await this.mock.supportsInterface.sendTransaction(this.target.address, DUMMY_ID);
expect(tx1.receipt.gasUsed).to.be.lessThan(120000); // 3*30k + 21k + some margin

const tx2 = await this.mock.getSupportedInterfaces.sendTransaction(
this.target.address,
[
DUMMY_ID,
DUMMY_ID_2,
DUMMY_ID_3,
DUMMY_UNSUPPORTED_ID,
DUMMY_UNSUPPORTED_ID_2,
],
);
expect(tx2.receipt.gasUsed).to.be.lessThan(250000); // (2+5)*30k + 21k + some margin
});
});

0 comments on commit 8fb5f57

Please sign in to comment.