Multiple Netlink Decode Paths Can Panic on Malformed or Undersized Input
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 68/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- go, linux
- Domain
- networking
Research direction
Start with the listed decode paths in expr/ct.go, expr/verdict.go, expr/log.go, expr/connlimit.go, set.go, chain.go, obj.go, and alignedbuff.go, then run expr/ct_timeout_poc_test.go. Done means malformed or undersized attributes return errors without panicking, and elementsFromMsg no longer produces spurious empty SetElements.
Written by the indexing model from the issue text.
Description
Description
Summary
Several decode paths consume kernel-controlled netlink attribute data without length validation, and one has a check-after-use bug. On malformed/undersized kernel input these panic instead of returning an error. Normal, healthy kernel replies provide correct lengths, so these are robustness/hardening issues rather than easily-triggered vulnerabilities. Worth fixing together for crash-safety.
1. expr/ct.go:416-418 — error checked AFTER decoder is dereferenced (nil deref)
decoder, err := netlink.NewAttributeDecoder(ad.Bytes())
decoder.ByteOrder = binary.BigEndian // deref before err is checked
if err != nil {
return err
}
NewAttributeDecoder returns (nil, err) when the input ends in a partial (<4-byte) attribute header (attribute.go available() returns errInvalidAttribute when len(b[i:]) < nlaHeaderLen).
Dereferencing decoder before checking err then crashes.
Every other similar call site in the repo checks err first; this one is the only inversion.
PoC (expr/ct_timeout_poc_test.go):
package expr
import (
"testing"
"github.com/mdlayher/netlink"
)
func TestCtTimeoutUnmarshalMalformedPanic(t *testing.T) {
data, err := netlink.MarshalAttributes([]netlink.Attribute{
{Type: NFTA_CT_TIMEOUT_DATA, Data: []byte{0xff}},
})
if err != nil {
t.Fatal(err)
}
ct := &CtTimeout{}
_ = ct.unmarshal(0, data)
}
Observed:
panic: runtime error: invalid memory address or nil pointer dereference
github.com/google/nftables/expr.(*CtTimeout).unmarshal ... expr/ct.go:417
2. Unbounded binary reads / slicing on ad.Bytes()
These call binaryutil.BigEndian.Uint* / binary.BigEndian.Uint* / slice an ad.Bytes() result without checking length; a short attribute panics (slice bounds / index out of range) instead of returning an error.
| Location | Code | Panics when |
|---|---|---|
expr/verdict.go:119 |
binaryutil.BigEndian.Uint32(nestedAD.Bytes()[4:8]) |
verdict data < 8 bytes |
expr/log.go:142 |
e.Data = data[:len(data)-1] |
empty NFTA_LOG_PREFIX → negative slice index |
expr/log.go:139,144,146,148,150 |
binaryutil.BigEndian.Uint16/32(data) |
short NFTA_LOG_* |
expr/connlimit.go:68,70 |
binaryutil.BigEndian.Uint32(ad.Bytes()) |
short attr |
set.go:828,830,849,862,883 |
binary.BigEndian.Uint32/64(ad.Bytes()) |
short attr |
chain.go:310 |
binaryutil.BigEndian.Uint32(ad.Bytes()) |
short NFTA_CHAIN_POLICY |
obj.go:234,247 |
msg.Data[0] / msg.Data[4:] |
empty message |
binaryutil.NativeEndian.Uint16/32/64 and binaryutil.Int32 additionally dereference &b[0] via unsafe and panic on empty/too-short slices (they are only safe when length is pre-verified, as in alignedbuff).
3. set.go:940-948 — spurious empty element (logic, not a panic)
for ad.Next() {
var elem SetElement
switch ad.Type() {
case unix.NFTA_LIST_ELEM:
ad.Do(elem.decode(fam))
}
elements = append(elements, elem) // appended for every attribute
}
elements = append(elements, elem) runs for every attribute, not just NFTA_LIST_ELEM, producing spurious empty SetElements.
4. alignedbuff.go:144-156 / :160-164 — String() / StringWithLength()
Have no bounds checks and can read OOB / panic, but are currently not reachable from production code (only used in tests). Future-facing hardening.
Impact
- Availability: process crash (panic) triggered only by malformed / undersized kernel input — not expected from a healthy kernel.
- Confidentiality / Integrity: none.
- Severity: low; these are robustness/code-quality issues. Fixes are trivial (check length / reorder error check).
Suggested fixes
ct.go:417: movedecoder.ByteOrderbelowif err != nil { return err }.- For the
Uint*/ slice call sites: either use the bounded decoder methods (ad.Uint32/64) or validatelen(...)before reading. elementsFromMsg: only append inside thecase NFTA_LIST_ELEM.
-Eneshan Erdoğan Karaca
- Dominant language
- Go
- Stars
- 1.4k
- Forks
- 187
- PR merge metrics
- No merged PRs in 30d
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from google/nftables
-
Difficulty 2/5 1-3 hours Newbie friendliness 83/100
-
New Tag release.Open
Difficulty 3/5 1-2 days Newbie friendliness 45/100
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
-
Difficulty 5/5 Over a week Newbie friendliness 25/100
-
Merge set elementsOpen
Difficulty 4/5 3-5 days Newbie friendliness 42/100
Similar issues
-
cleanup hacktoberfest priority: low skill: good first issue testing
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
hyperledger-labs/fabric-smart-client#2197 ·
Maintainers usually reply within 1 day
-
New term request
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
geneontology/go-ontology#32791 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 69/100
aws/amazon-ecs-agent#5177 ·
Maintainers usually reply within 2 days
-
release-followup
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
Maintainers usually reply within 1 day
-
kind/bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day