Skip to content

Commit

Permalink
New Published Rules - 1512543916_personal_org.missing-self-transfer-c…
Browse files Browse the repository at this point in the history
…heck-ercx (#3373)

* add 1512543916_personal_org/missing-self-transfer-check-ercx.yaml

* add 1512543916_personal_org/missing-self-transfer-check-ercx.sol

* move missing-self-transfer-check-ercx to solidity folder

* move missing-self-transfer-check-ercx to solidity folder

---------

Co-authored-by: MarkLee131 <1512543916@qq.com>
Co-authored-by: Vasilii <inkz@xakep.ru>
  • Loading branch information
3 people authored May 8, 2024
1 parent b6d791b commit 9958265
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 0 deletions.
90 changes: 90 additions & 0 deletions solidity/security/missing-self-transfer-check-ercx.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
function _update(address from, address to, uint256 value, bool mint) internal virtual {
uint256 fromBalance = _balances[from];
uint256 toBalance = _balances[to];
if (fromBalance < value) {
revert ERC20InsufficientBalance(from, fromBalance, value);
}

//No need to adjust balances when transfer is to self, prevent self NFT-grind

unchecked {
// Overflow not possible: value <= fromBalance <= totalSupply.
// ruleid: missing-self-transfer-check-ercx
_balances[from] = fromBalance - value;
// ruleid: missing-self-transfer-check-ercx
_balances[to] = toBalance + value;


if(mint) {
// Skip burn for certain addresses to save gas
bool wlf = whitelist[from];
if (!wlf) {
uint256 tokens_to_burn = (fromBalance / tokensPerNFT) - ((fromBalance - value) / tokensPerNFT);
if(tokens_to_burn > 0)
_burnBatch(from, tokens_to_burn);
}

// Skip minting for certain addresses to save gas
if (!whitelist[to]) {
if(easyLaunch == 1 && wlf && from == owner()) {
//auto-initialize first (assumed) LP
whitelist[to] = true;
easyLaunch = 2;
} else {
uint256 tokens_to_mint = ((toBalance + value) / tokensPerNFT) - (toBalance / tokensPerNFT);
if(tokens_to_mint > 0)
_mintWithoutCheck(to, tokens_to_mint);
}
}
}
}

emit Transfer(from, to, value);
}


function _update(address from, address to, uint256 value, bool mint) internal virtual {
uint256 fromBalance = _balances[from];
uint256 toBalance = _balances[to];
if (fromBalance < value) {
revert ERC20InsufficientBalance(from, fromBalance, value);
}

//No need to adjust balances when transfer is to self, prevent self NFT-grind
if (from != to) {
unchecked {
// Overflow not possible: value <= fromBalance <= totalSupply.
//ok: missing-self-transfer-check-ercx
_balances[from] = fromBalance - value;

// Overflow not possible: balance + value is at most totalSupply, which we know fits into a uint256.
//ok: missing-self-transfer-check-ercx
_balances[to] = toBalance + value;
}

if(mint) {
// Skip burn for certain addresses to save gas
bool wlf = whitelist[from];
if (!wlf) {
uint256 tokens_to_burn = (fromBalance / tokensPerNFT) - ((fromBalance - value) / tokensPerNFT);
if(tokens_to_burn > 0)
_burnBatch(from, tokens_to_burn);
}

// Skip minting for certain addresses to save gas
if (!whitelist[to]) {
if(easyLaunch == 1 && wlf && from == owner()) {
//auto-initialize first (assumed) LP
whitelist[to] = true;
easyLaunch = 2;
} else {
uint256 tokens_to_mint = ((toBalance + value) / tokensPerNFT) - (toBalance / tokensPerNFT);
if(tokens_to_mint > 0)
_mintWithoutCheck(to, tokens_to_mint);
}
}
}
}

emit Transfer(from, to, value);
}
43 changes: 43 additions & 0 deletions solidity/security/missing-self-transfer-check-ercx.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
rules:
- id: missing-self-transfer-check-ercx
languages:
- solidity
message: >-
Missing check for 'from' and 'to' being the same before updating balances
could lead to incorrect balance manipulation on self-transfers.
Include a check to ensure 'from' and 'to' are not the same before updating balances to prevent balance manipulation during self-transfers.
severity: ERROR
metadata:
category: security
technology:
- blockchain
- solidity
cwe: 'CWE-682: Incorrect Calculation'
subcategory:
- vuln
confidence: HIGH
likelihood: HIGH
impact: HIGH
owasp:
- A7:2021 Identification and Authentication Failures
references:
- https://blog.verichains.io/p/miner-project-attacked-by-vulnerabilities
- https://x.com/shoucccc/status/1757777764646859121
patterns:
- pattern-either:
- pattern: |
_balances[$FROM] = $FROM_BALANCE - value;
- pattern: |
_balances[$TO] = $TO_BALANCE + value;
- pattern-not-inside: |
if ($FROM != $TO) {
...
_balances[$FROM] = $FROM_BALANCE - value;
...
_balances[$TO] = $TO_BALANCE + value;
...
}
- pattern-inside: |
function _update(address $FROM, address $TO, uint256 value, bool mint) internal virtual {
...
}

0 comments on commit 9958265

Please sign in to comment.