|
| 1 | +## Detect problematic JSON tags with dash prefix |
| 2 | + |
| 3 | +* [Playground Link](/playground.html#eyJtb2RlIjoiQ29uZmlnIiwibGFuZyI6ImdvIiwicXVlcnkiOiJgJFRBR2AiLCJyZXdyaXRlIjoiIiwic3RyaWN0bmVzcyI6InNtYXJ0Iiwic2VsZWN0b3IiOiIiLCJjb25maWciOiJpZDogdW5tYXJzaGFsLXRhZy1pcy1kYXNoXG5zZXZlcml0eTogZXJyb3Jcbm1lc3NhZ2U6IFN0cnVjdCBmaWVsZCBjYW4gYmUgZGVjb2RlZCB3aXRoIHRoZSBgLWAga2V5IGJlY2F1c2UgdGhlIEpTT04gdGFnXG4gIHN0YXJ0cyB3aXRoIGEgYC1gIGJ1dCBpcyBmb2xsb3dlZCBieSBhIGNvbW1hLlxucnVsZTpcbiAgcGF0dGVybjogJ2AkVEFHYCdcbiAgaW5zaWRlOlxuICAgIGtpbmQ6IGZpZWxkX2RlY2xhcmF0aW9uXG5jb25zdHJhaW50czpcbiAgVEFHOiBcbiAgICByZWdleDoganNvbjpcIi0sLipcIiIsInNvdXJjZSI6InBhY2thZ2UgbWFpblxuXG50eXBlIFRlc3RTdHJ1Y3QxIHN0cnVjdCB7XG5cdC8vIG9rOiB1bm1hcnNoYWwtdGFnLWlzLWRhc2hcblx0QSBzdHJpbmcgYGpzb246XCJpZFwiYFxufVxuXG50eXBlIFRlc3RTdHJ1Y3QyIHN0cnVjdCB7XG5cdC8vIHJ1bGVpZDogdW5tYXJzaGFsLXRhZy1pcy1kYXNoXG5cdEIgc3RyaW5nIGBqc29uOlwiLSxvbWl0ZW1wdHlcImBcbn1cblxudHlwZSBUZXN0U3RydWN0MyBzdHJ1Y3Qge1xuXHQvLyBydWxlaWQ6IHVubWFyc2hhbC10YWctaXMtZGFzaFxuXHRDIHN0cmluZyBganNvbjpcIi0sMTIzXCJgXG59XG5cbnR5cGUgVGVzdFN0cnVjdDQgc3RydWN0IHtcblx0Ly8gcnVsZWlkOiB1bm1hcnNoYWwtdGFnLWlzLWRhc2hcblx0RCBzdHJpbmcgYGpzb246XCItLFwiYFxufSJ9) |
| 4 | + |
| 5 | +### Description |
| 6 | + |
| 7 | +This rule detects a security vulnerability in Go's JSON unmarshaling. When a struct field has a JSON tag that starts with `-,`, it can be unexpectedly unmarshaled with the `-` key. |
| 8 | + |
| 9 | +According to the [Go documentation](https://pkg.go.dev/encoding/json#Marshal), if the field tag is `-`, the field should be omitted. However, a field with name `-` can still be unmarshaled using the tag `-,`. |
| 10 | + |
| 11 | +This creates a security issue where developers think they are preventing a field from being unmarshaled (like `IsAdmin` in authentication), but attackers can still set that field by providing the `-` key in JSON input. |
| 12 | + |
| 13 | +### Example of the vulnerability |
| 14 | + |
| 15 | +```go |
| 16 | +type User struct { |
| 17 | + Username string `json:"username,omitempty"` |
| 18 | + Password string `json:"password,omitempty"` |
| 19 | + IsAdmin bool `json:"-,omitempty"` // Intended to prevent marshaling |
| 20 | +} |
| 21 | + |
| 22 | +// This still works and sets IsAdmin to true! |
| 23 | +json.Unmarshal([]byte(`{"-": true}`), &user) |
| 24 | +``` |
| 25 | + |
| 26 | +### YAML |
| 27 | + |
| 28 | +```yaml |
| 29 | +id: unmarshal-tag-is-dash |
| 30 | +severity: error |
| 31 | +message: Struct field can be decoded with the `-` key because the JSON tag |
| 32 | + starts with a `-` but is followed by a comma. |
| 33 | +rule: |
| 34 | + pattern: '`$TAG`' |
| 35 | + inside: |
| 36 | + kind: field_declaration |
| 37 | +constraints: |
| 38 | + TAG: |
| 39 | + regex: json:"-,.*" |
| 40 | +``` |
| 41 | +
|
| 42 | +### Example |
| 43 | +
|
| 44 | +<!-- highlight matched code in curly-brace {lineNum} --> |
| 45 | +```go{5,10,15,20} |
| 46 | +package main |
| 47 | + |
| 48 | +type TestStruct1 struct { |
| 49 | + // ok: unmarshal-tag-is-dash |
| 50 | + A string `json:"id"` |
| 51 | +} |
| 52 | + |
| 53 | +type TestStruct2 struct { |
| 54 | + // ruleid: unmarshal-tag-is-dash |
| 55 | + B string `json:"-,omitempty"` |
| 56 | +} |
| 57 | + |
| 58 | +type TestStruct3 struct { |
| 59 | + // ruleid: unmarshal-tag-is-dash |
| 60 | + C string `json:"-,123"` |
| 61 | +} |
| 62 | + |
| 63 | +type TestStruct4 struct { |
| 64 | + // ruleid: unmarshal-tag-is-dash |
| 65 | + D string `json:"-,"` |
| 66 | +} |
| 67 | +``` |
| 68 | + |
| 69 | +### Fix |
| 70 | + |
| 71 | +To properly omit a field from JSON marshaling/unmarshaling, use just `-` without a comma: |
| 72 | + |
| 73 | +```go |
| 74 | +type User struct { |
| 75 | + Username string `json:"username,omitempty"` |
| 76 | + Password string `json:"password,omitempty"` |
| 77 | + IsAdmin bool `json:"-"` // Correctly prevents marshaling/unmarshaling |
| 78 | +} |
| 79 | +``` |
| 80 | + |
| 81 | +### Contributed by |
| 82 | + |
| 83 | +Inspired by [Trail of Bits blog post](https://blog.trailofbits.com/2025/06/17/unexpected-security-footguns-in-gos-parsers/) and their [public Semgrep rule](https://semgrep.dev/playground/r/trailofbits.go.unmarshal-tag-is-dash). |
0 commit comments