Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Kraken rules #63

Merged
merged 8 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ $ semgrep --config /path/to/semgrep-rules/hanging-goroutine.yml -o leaks.txt'

| ID | Playground | Impact | Confidence | Description |
| -- | :--------: | :----: | :--------: | ----------- |
| [eth-rpc-tracetransaction](go/eth-rpc-tracetransaction.yaml) | [🛝🔗](https://semgrep.dev/playground/r/trailofbits.go.eth-rpc-tracetransaction.eth-rpc-tracetransaction) | 🟥 | 🌕 | Detects attempts to extract trace information from an EVM transaction or block |
| [eth-txreceipt-status](go/eth-txreceipt-status.yaml) | [🛝🔗](https://semgrep.dev/playground/r/trailofbits.go.eth-txreceipt-status.eth-txreceipt-status) | 🟥 | 🌕 | Detects when a transaction receipt's status is read |
| [hanging-goroutine](go/hanging-goroutine.yaml) | [🛝🔗](https://semgrep.dev/playground/r/trailofbits.go.hanging-goroutine.hanging-goroutine) | 🟩 | 🌗 | Goroutine leaks |
| [invalid-usage-of-modified-variable](go/invalid-usage-of-modified-variable.yaml) | [🛝🔗](https://semgrep.dev/playground/r/trailofbits.go.invalid-usage-of-modified-variable.invalid-usage-of-modified-variable) | 🟧 | 🌘 | Possible unintentional assignment when an error occurs |
| [iterate-over-empty-map](go/iterate-over-empty-map.yaml) | [🛝🔗](https://semgrep.dev/playground/r/trailofbits.go.iterate-over-empty-map.iterate-over-empty-map) | 🟩 | 🌗 | Probably redundant iteration over an empty map |
Expand Down
48 changes: 48 additions & 0 deletions go/eth-rpc-tracetransaction.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package main

import (
"fmt"
"io"
"net/http"
"strings"
)

func Test() {
// ruleid: eth-rpc-tracetransaction
data, err := client.TraceTransaction(ctx, "hash", nil)
// ruleid: eth-rpc-tracetransaction
data, err := client.TraceBlockByNumber(ctx, 5, nil)
// ruleid: eth-rpc-tracetransaction
data, err := client.TraceBlockByHash(ctx, []byte{0x05}, nil)
// ruleid: eth-rpc-tracetransaction
data, err := client.TraceBlock(ctx, []byte{0x05}, nil)
// ruleid: eth-rpc-tracetransaction
data, err := client.TraceChain(ctx, 5, nil)

url := "https://eth-mainnet.g.alchemy.com/v2/docs-demo"
// ruleid: eth-rpc-tracetransaction
payload := strings.NewReader("{\"id\":1,\"jsonrpc\":\"2.0\",\"method\":\"trace_transaction\",\"params\":[\"0x8fc90a6c3ee3001cdcbbb685b4fbe67b1fa2bec575b15b0395fea5540d0901ae\"]}")

req, _ := http.NewRequest("POST", url, payload)

req.Header.Add("accept", "application/json")
req.Header.Add("content-type", "application/json")

res, _ := http.DefaultClient.Do(req)

defer res.Body.Close()
body, _ := io.ReadAll(res.Body)

fmt.Println(string(body))

// ok: eth-rpc-tracetransaction
data, err := client.TraceSomething(ctx, 5, nil)
// ok: eth-rpc-tracetransaction
data, err := client.TraceTransaction(ctx, "hash")
// ok: eth-rpc-tracetransaction
data, err := client.TraceBlockByNumber(ctx, 5)
// ok: eth-rpc-tracetransaction
data, err := client.TraceBlockByHash(ctx, []byte{0x05})
// ok: eth-rpc-tracetransaction
data, err := client.TraceBlock(ctx, []byte{0x05})
}
43 changes: 43 additions & 0 deletions go/eth-rpc-tracetransaction.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
rules:
- id: eth-rpc-tracetransaction
message: >-
Using built-in transaction tracers can be dangerous if measures are not taken to filter out reverted call frames.
Review the related code to ensure the following properties:
1. Reverted call frames and their associated subtraces are filtered out from any analysis.
2. The transaction being traced is from a finalized block.
severity: WARNING
languages: [go]
metadata:
category: security
technology: [ethereum, blockchain, geth]
subcategory: [audit]
cwe: "CWE-1284: Improper Validation of Specified Quantity in Input"
confidence: LOW
impact: HIGH
likelihood: MEDIUM
description: Detects attempts to extract trace information from an EVM transaction or block. In exchange or bridge applications, extra logic must be implemented encapsulating these endpoints to prevent the values transferred during reverted call frames from being counted.
references:
- https://blog.trailofbits.com/2023/08/23/the-engineers-guide-to-blockchain-finality/

pattern-either:
# Calls directly into Geth's API
- pattern: $RECEIVER.TraceTransaction($CTX, $FILTER, $TRACECONF)
- pattern: $RECEIVER.TraceBlockByNumber($CTX, $FILTER, $TRACECONF)
- pattern: $RECEIVER.TraceBlockByHash($CTX, $FILTER, $TRACECONF)
- pattern: $RECEIVER.TraceBlock($CTX, $FILTER, $TRACECONF)
- pattern: $RECEIVER.TraceChain($CTX, ...)
# RPC calls over HTTP API to geth/node provider
bsamuels453 marked this conversation as resolved.
Show resolved Hide resolved
GrosQuildu marked this conversation as resolved.
Show resolved Hide resolved
- pattern-regex: .*debug_traceBlock.*
- pattern-regex: .*debug_traceTransaction.*
- pattern-regex: .*debug_traceCall.*
- pattern-regex: .*debug_traceBlockByNumber.*
- pattern-regex: .*debug_traceBlockByHash.*
# RPC calls over HTTP API to non-geth client/node provider
- pattern-regex: .*trace_block.*
- pattern-regex: .*trace_transaction.*
- pattern-regex: .*trace_replayBlockTransactions.*
- pattern-regex: .*trace_replayTransaction.*
- pattern-regex: .*trace_filter.*
- pattern-regex: .*trace_call.*
- pattern-regex: .*trace_callMany.*
- pattern-regex: .*trace_get.*
18 changes: 18 additions & 0 deletions go/eth-txreceipt-status.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package main

import "github.com/ethereum/go-ethereum/core/types"

type Thing struct {
Id Int
Status bool
}

func Test() {
var debug Receipt
// ruleid: eth-txreceipt-status
a := debug.Status

var debug2 Thing
// ok: eth-txreceipt-status
b := debug2.Status
}
26 changes: 26 additions & 0 deletions go/eth-txreceipt-status.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
rules:
- id: eth-txreceipt-status
message: >-
A transaction receipt's status is inspected using `$RECEIVER.Status()`. For bridges and exchanges, this is a high-risk pattern because even though the transaction was successful, calls within the transaction may have failed. Review the related code to ensure the following properties:
1. The receipt's success is not being used as a verification measure.
2. The transaction being inspected is from a finalized block.
severity: WARNING
languages: [go]
metadata:
category: security
confidence: LOW
impact: HIGH
likelihood: MEDIUM
technology: [ethereum, blockchain, geth]
subcategory: [audit]
cwe: "CWE-437: Incomplete Model of Endpoint Features"
description: Detects when a transaction receipt's status is read
references:
- https://blog.trailofbits.com/2023/08/23/the-engineers-guide-to-blockchain-finality/

patterns:
- pattern-inside: |
import "github.com/ethereum/go-ethereum/core/types"
...
- pattern: |
($RECEIVER : Receipt).Status
Loading