# Semgrep rules for smart contracts

In this repository you can find [semgrep](https://semgrep.dev/) rules that look for patterns of vulnerabilities in smart contracts based on actual DeFi exploits as well as gas optimization rules that can be used as a part of the CI pipeline. The rules are part of the semgrep registry under [p/smart-contracts](https://semgrep.dev/p/smart-contracts).

## Disclaimer

Currently semgrep supports [Solidity](https://semgrep.dev/docs/language-support/) in `experimental` mode. Some of the rules may not work until Solidity is in `beta` at least.

## Scanning

> **Important:** Some of the rules utilize the [taint mode](https://semgrep.dev/docs/writing-rules/data-flow/taint-mode), which is restricted to the same function in the open-source version of semgrep. To take advantage of intra-procedural taint analysis, you must include the `--pro` flag with each command. Please note that this requires semgrep Pro.

1) By cloning the repository:

  ```shell
  $ semgrep --config solidity/security path/to/your/project
  ```

2) By using [semgrep registry](https://semgrep.dev/r):

  ```shell
  $ semgrep --config p/smart-contracts path/to/your/project
  ```

3) In your CI:

Create `run-semgrep.yaml` in `.github/workflows` with the following contents:
<details>
  <summary>run-semgrep.yaml</summary>
  
```yaml
# Name of this GitHub Actions workflow.
name: Run Semgrep

on:
  # Scan changed files in PRs (diff-aware scanning):
  pull_request: {}
  # On-demand 
  workflow_dispatch: {}

jobs:
  semgrep:
    # User-definable name of this GitHub Actions job:
    name: Scan
    # If you are self-hosting, change the following `runs-on` value: 
    runs-on: ubuntu-latest

    container:
      # A Docker image with Semgrep installed. Do not change this.
      image: returntocorp/semgrep

    # Skip any PR created by dependabot to avoid permission issues:
    if: (github.actor != 'dependabot[bot]')

    steps:
      # Fetch project source with GitHub Actions Checkout.
      - uses: actions/checkout@v3
      # Fetch semgrep rules
      - name: Fetch semgrep rules
        uses: actions/checkout@v3
        with:
          repository: decurity/semgrep-smart-contracts
          path: rules
      # Run security and gas optimization rules
      - run: semgrep ci --sarif --output=semgrep.sarif || true
        env:
           SEMGREP_RULES: rules/solidity/security rules/solidity/performance
      # Upload findings to GitHub Advanced Security Dashboard
      - name: Upload findings to GitHub Advanced Security Dashboard
        uses: github/codeql-action/upload-sarif@v2
        with:
          sarif_file: semgrep.sarif
        if: always()
```
</details>

## Testing

Each rule is accompanied by an actual vulnerable source code that was targeted by an exploit. Vulnerable lines are marked with `// ruleid: ...`

In case a rule is not yet supported by semgrep, you will find `// todoruleid: ...`

Run tests: 

```shell
$ semgrep --test solidity
```

Validate rules: 

```shell
$ semgrep --validate --config solidity
```

Feel free to submit any issues with the precision and quality of the rules!

## Security Rules

Rule ID | Targets | Description
--- | --- | ---
compound-borrowfresh-reentrancy | Compound, Ola Finance, Hundred Finance, Agave | Function borrowFresh() in Compound performs state update after doTransferOut()
compound-sweeptoken-not-restricted | TUSD, Compound | Function sweepToken is allowed to be called by anyone
erc20-public-transfer | Creat Future | Custom ERC20 implementation exposes _transfer() as public
erc20-public-burn | HospoWise | Anyone can burn tokens of other accounts
erc677-reentrancy | Ola Finance | ERC677 callAfterTransfer() reentrancy
erc777-reentrancy | Bacon Protocol | ERC777 tokensReceived() reentrancy
erc721-reentrancy | Hype Bears | ERC721 onERC721Received() reentrancy
erc721-arbitrary-transferfrom | Distortion Genesis | Custom ERC721 implementation lacks access control checks in _transfer()
gearbox-tokens-path-confusion | Gearbox | UniswapV3 adapter implemented incorrect extraction of path parameters
keeper-network-oracle-manipulation | Inverse Finance | Keep3rV2.current() call has high data freshness, but it has low security, an exploiter simply needs to manipulate 2 data points to be able to impact the feed.
basic-oracle-manipulation | Onering Finance, Deus Finance | getSharePrice() can be manipulated via flashloan
redacted-cartel-custom-approval-bug | Redacted Cartel | transferFrom() can steal allowance of other accounts
rigoblock-missing-access-control | RigoBlock | setMultipleAllowances() is missing onlyOwner modifier
oracle-price-update-not-restricted | Rikkei Finance, Aave | Oracle price data can be submitted by anyone
superfluid-ctx-injection | Superfluid | A specially crafted calldata may be used to impersonate other accounts
tecra-coin-burnfrom-bug | Tecra Coin | Parameter "from" is checked at incorrect position in "_allowances" mapping
arbitrary-low-level-call | Auctus Options, Starstream Finance, BasketDAO, Li Finance | An attacker may perform call() to an arbitrary address with controlled calldata
sense-missing-oracle-access-control | Sense Finance | Oracle update is not restricted in onSwap(), rule by [Arbaz Kiraak](https://twitter.com/ArbazKiraak)
proxy-storage-collision | Audius | Proxy declares a state var that may override a storage slot of the implementation
uniswap-callback-not-protected | Generic | Uniswap callback is not protected
encode-packed-collision | Generic | Hash collision with variable length arguments in abi.encodePacked
openzeppelin-ecdsa-recover-malleable | OpenZeppelin | Potential signature malleability
BETA: basic-arithmetic-underflow | Umbrella Network, Remittance Token | Possible arithmetic underflow
unrestricted-transferownership | Ragnarok Online Invasion | Contract ownership can be transfered by anyone
msg-value-multicall | Sushiswap | Function with constant msg.value can be called multiple times
no-bidi-characters | Generic | The code must not contain any of Unicode Direction Control Characters
delegatecall-to-arbitrary-address | Generic | An attacker may perform delegatecall() to an arbitrary address.
incorrect-use-of-blockhash | Generic | blockhash(block.number) and blockhash(block.number + N) always returns 0.
accessible-selfdestruct | Generic | Contract can be destructed by anyone in $FUNC
no-slippage-check| Generic|  No slippage check in a Uniswap v2/v3 trade
balancer-readonly-reentrancy-getrate | Balancer | getRate() call on a Balancer pool is not protected from the read-only reentrancy.
balancer-readonly-reentrancy-getpooltokens | Balancer | getPoolTokens() call on a Balancer pool is not protected from the read-only reentrancy.
curve-readonly-reentrancy | Curve | get_virtual_price() call on a Curve pool is not protected from the read-only reentrancy.
public-transfer-fees-supporting-tax-tokens | LeetSwap | public _transferFeesSupportingTaxTokens() without any modificators 
olympus-dao-staking-incorrect-call-order | OlympusDAO, FloorDAO, Heavens Gate, Jump Farm, QuantumWN | The order of calling the transferFrom() and rebase() functions is incorrect in Olympus DAO forks
compound-precision-loss | Hundred Finance, Midas Finance, Onyx Protocol | In Compound forks if there is a market with totalSupply = 0 and collateralFactor != 0 a precision loss attack is possible if redeemAmount is taken from the arguments of redeemFresh()
thirdweb-vulnerability | Swopple Token, TIME Token, NAME Token, HXA Token | In contracts that support Multicall and ERC2771Context an Arbitrary Address Spoofing attack is possible
exact-balance-check | Generic | Testing the balance of an account as a basis for some action has risks associated with unexpected receipt of ether or another token, including tokens deliberately transfered to cause such tests to fail, as an MEV attack.
missing-assignment | Generic | Meaningless statement that does not change any values could be a sign of missed security checks or other important changes.
oracle-uses-curve-spot-price | UwU | Oracle uses the get_p() Curve pool function which can be manipulated via flashloan to calculate the asset price
bad-transferfrom-access-control | Generic | Funds approved by users can be stolen because of improper access control to a transferFrom function
uniswap-v4-callback-not-protected | Uniswap v4 | Uniswap v4 callback is not protected

## Gas Optimization Rules

Rule ID | Description
--- | ---
array-length-outside-loop | Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop.
init-variables-with-default-value | Explicitly initializing a variable with its default value costs unnecessary gas.
state-variable-read-in-a-loop | Replace state variable reads and writes within loops with local variable reads and writes.
unnecessary-checked-arithmetic-in-loop | A lot of times there is no risk that the loop counter can overflow. Using Solidity's unchecked block saves the overflow checks.
use-custom-error-not-require | Consider using custom errors as they are more gas efficient while allowing developers to describe the error in detail using NatSpec.
use-multiple-require | Using multiple require statements is cheaper than using && multiple check combinations. 
use-nested-if | Using nested is cheaper than using && multiple check combinations.
use-prefix-decrement-not-postfix | The prefix decrement expression is cheaper in terms of gas.
use-prefix-increment-not-postfix | The prefix increment expression is cheaper in terms of gas.
use-short-revert-string | Shortening revert strings to fit in 32 bytes will decrease gas costs for deployment and gas costs when the revert condition has been met.
non-payable-constructor | Consider making costructor payable to save gas.
non-optimal-variables-swap | Consider swapping variables using `($VAR1, $VAR2) = ($VAR2, $VAR1)` to save gas.
inefficient-state-variable-increment | <x> += <y> costs more gas than <x> = <x> + <y> for state variables.

## Best Practices Rules

Rule ID | Description
--- | ---
use-abi-encodecall-instead-of-encodewithselector | To guarantee arguments type safety it is recommended to use `abi.encodeCall` instead of `abi.encodeWithSelector`.
use-ownable2step | By demanding that the receiver of the owner permissions actively accept via a contract call of its own, `Ownable2Step` and `Ownable2StepUpgradeable` prevent the contract ownership from accidentally being transferred to an address that cannot handle it.

## Solana Rules
Rule ID | Description
--- | ---
solana-arbitrary-program-call | An attacker may be able to invoke arbitrary programs without address validations
solana-insecure-account-closing | Writing the CLOSED_ACCOUNT_DISCRIMINATOR to a closed account is crucial to prevent the reuse of the account within the same transaction

## Cairo Rules
Rule ID | Description
--- | ---
lack-of-error-message  | Error message is missing in the assert statement
tx-origin-authentication | Using `account_contract_address` for authentication is insecure. Use `get_caller_address` or an appropriate method for verifying users.
view-fn-mutable-state | View function should not be able to modify state
view-fn-writes | View function should not write to the state
zero-division | Possible division by zero