Add Kraken rules (#63)

* add kraken rules

* update readme

* add negative tests

* more efficient eth-txreceipt-status.yaml

* clear formatting

* fix prettier

* add tests

* add recommended changes

---------

Co-authored-by: GrosQuildu <[email protected]>

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 |

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})
+}

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.*

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
+}

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