Skip to content
This repository was archived by the owner on May 9, 2024. It is now read-only.

Commit b929c98

Browse files
mpetrun5P1sar
andauthored
Check depositor when burning ERC721 tokens (#687)
* burn fix * Release v2.1.4 * Fix tests --------- Co-authored-by: Kirill Pisarev <[email protected]>
1 parent 2f29dd7 commit b929c98

File tree

4 files changed

+15
-5
lines changed

4 files changed

+15
-5
lines changed

contracts/ERC721Safe.sol

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,9 @@ contract ERC721Safe {
5555
@param tokenAddress Address of ERC721 to burn.
5656
@param tokenID ID of token to burn.
5757
*/
58-
function burnERC721(address tokenAddress, uint256 tokenID) internal {
58+
function burnERC721(address tokenAddress, address owner, uint256 tokenID) internal {
5959
ERC721MinterBurnerPauser erc721 = ERC721MinterBurnerPauser(tokenAddress);
60+
require(erc721.ownerOf(tokenID) == owner, 'Burn not from owner');
6061
erc721.burn(tokenID);
6162
}
6263

contracts/handlers/ERC721Handler.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ contract ERC721Handler is IDepositExecute, HandlerHelpers, ERC721Safe {
5858
}
5959

6060
if (_burnList[tokenAddress]) {
61-
burnERC721(tokenAddress, tokenID);
61+
burnERC721(tokenAddress, depositer, tokenID);
6262
} else {
6363
lockERC721(tokenAddress, depositer, address(this), tokenID);
6464
}

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@chainsafe/chainbridge-contracts",
3-
"version": "2.1.3",
3+
"version": "2.1.4",
44
"description": "",
55
"main": "dist/index.js",
66
"repository": "https://github.com/ChainSafe/chainbridge-solidity.git",

test/handlers/erc721/depositBurn.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Copyright 2020 ChainSafe Systems
33
* SPDX-License-Identifier: LGPL-3.0-only
44
*/
5-
5+
66
const TruffleAssert = require('truffle-assertions');
77

88
const Helpers = require('../../helpers');
@@ -48,7 +48,7 @@ contract('ERC721Handler - [Deposit Burn ERC721]', async (accounts) => {
4848
ERC721HandlerContract.new(BridgeInstance.address).then(instance => ERC721HandlerInstance = instance),
4949
ERC721MintableInstance1.mint(depositerAddress, tokenID, "")
5050
]);
51-
51+
5252
await Promise.all([
5353
ERC721MintableInstance1.approve(ERC721HandlerInstance.address, tokenID, { from: depositerAddress }),
5454
BridgeInstance.adminSetResource(ERC721HandlerInstance.address, resourceID1, ERC721MintableInstance1.address),
@@ -89,4 +89,13 @@ contract('ERC721Handler - [Deposit Burn ERC721]', async (accounts) => {
8989
ERC721MintableInstance1.ownerOf(tokenID),
9090
'ERC721: owner query for nonexistent token');
9191
});
92+
it('depositAmount of ERC721MintableInstance1 tokens should NOT burn from NOT token owner', async () => {
93+
await TruffleAssert.reverts(BridgeInstance.deposit(
94+
domainID,
95+
resourceID1,
96+
depositData,
97+
{ from: accounts[3] }
98+
),
99+
'Burn not from owner');
100+
});
92101
});

0 commit comments

Comments
 (0)