diff --git a/README.md b/README.md index 2d95df4..f5ef7a2 100644 --- a/README.md +++ b/README.md @@ -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 | diff --git a/go/eth-rpc-tracetransaction.go b/go/eth-rpc-tracetransaction.go new file mode 100644 index 0000000..b61cbc9 --- /dev/null +++ b/go/eth-rpc-tracetransaction.go @@ -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}) +} diff --git a/go/eth-rpc-tracetransaction.yaml b/go/eth-rpc-tracetransaction.yaml new file mode 100644 index 0000000..2bda9d5 --- /dev/null +++ b/go/eth-rpc-tracetransaction.yaml @@ -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 + - 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.* diff --git a/go/eth-txreceipt-status.go b/go/eth-txreceipt-status.go new file mode 100644 index 0000000..af39fd2 --- /dev/null +++ b/go/eth-txreceipt-status.go @@ -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 +} diff --git a/go/eth-txreceipt-status.yaml b/go/eth-txreceipt-status.yaml new file mode 100644 index 0000000..0c6487a --- /dev/null +++ b/go/eth-txreceipt-status.yaml @@ -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