-
Notifications
You must be signed in to change notification settings - Fork 277
/
self-transfer.sol
98 lines (79 loc) · 3.07 KB
/
self-transfer.sol
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;
import "forge-std/Test.sol";
/*
Name: Missing Check for Self-Transfer Allows Funds to be Lost
Description:
The vulnerability in the code stems from the absence of a check to prevent self-transfers.
This oversight allows the transfer function to erroneously transfer funds to the same address.
Consequently, funds are lost as the code fails to deduct the transferred amount from the sender's balance.
This vulnerability undermines the correctness of fund transfers within the contract and poses a risk
to the integrity of user balances.
Mitigation:
Add condition to prevent transfer between same addresses
REF:
https://twitter.com/1nf0s3cpt/status/1679373800327241728
https://github.com/code-423n4/2022-10-traderjoe-findings/issues/299
https://www.immunebytes.com/blog/bzxs-security-focused-relaunch-followed-by-a-hack-how/
*/
contract ContractTest is Test {
SimpleBank VSimpleBankContract;
FixedSimpleBank FixedSimpleBankContract;
function setUp() public {
VSimpleBankContract = new SimpleBank();
FixedSimpleBankContract = new FixedSimpleBank();
}
function testSelfTransfer() public {
VSimpleBankContract.transfer(address(this), address(this), 10000);
VSimpleBankContract.transfer(address(this), address(this), 10000);
VSimpleBankContract.balanceOf(address(this));
/*
unchecked {
_balances[_id][Alice] = 10000 - 10000;
_balances[_id][Alice] = 10000 + 10000;
total balance of [Alice] = 20000
}
*/
}
function testFixedSelfTransfer() public {
vm.expectRevert("Cannot transfer funds to the same address.");
FixedSimpleBankContract.transfer(address(this), address(this), 10000);
}
receive() external payable {}
}
contract SimpleBank {
mapping(address => uint256) private _balances;
function balanceOf(address _account) public view virtual returns (uint256) {
return _balances[_account];
}
function transfer(address _from, address _to, uint256 _amount) public {
// not check self-transfer
uint256 _fromBalance = _balances[_from];
uint256 _toBalance = _balances[_to];
unchecked {
_balances[_from] = _fromBalance - _amount;
_balances[_to] = _toBalance + _amount;
}
}
}
contract FixedSimpleBank {
mapping(address => uint256) private _balances;
function balanceOf(address _account) public view virtual returns (uint256) {
return _balances[_account];
}
function transfer(address _from, address _to, uint256 _amount) public {
//Mitigation
require(_from != _to, "Cannot transfer funds to the same address.");
uint256 _fromBalance = _balances[_from];
uint256 _toBalance = _balances[_to];
unchecked {
_balances[_from] = _fromBalance - _amount;
_balances[_to] = _toBalance + _amount;
/*
Another mitigation
_balances[_id][_from] -= _amount;
_balances[_id][_to] += _amount;
*/
}
}
}