Skip to content

badger: fix index-out-of-range panic on tags shorter than 2 elements - #76

Open
mauromoro wants to merge 1 commit into
fiatjaf:masterfrom
mauromoro:fix/badger-index-panic-on-short-tags
Open

mauromoro wants to merge 1 commit into
fiatjaf:masterfrom
mauromoro:fix/badger-index-panic-on-short-tags

Conversation

@mauromoro

Copy link
Copy Markdown

Summary

  • A tag with fewer than 2 elements (e.g. NIP-70's bare ["-"] protected-event marker) has no tag[1]. The len(tag) < 2 check correctly short-circuits the rest of its compound OR condition, but the branch it falls into then unconditionally evaluates tag[1] as a call argument to IndexLongerTag — Go evaluates call arguments before the call, so this panics with an out-of-range index before IndexLongerTag's own body (which does handle a short/empty value safely) ever runs.
  • Not hypothetical: once IndexLongerTag is configured, it's wired for every event a relay stores, not just ones it authors — so any client publishing any event with a bare single-element tag crashes the whole process.
  • Fixed by checking len(tag) < 2 on its own, before ever touching tag[1], rather than folded into the compound condition that still (correctly) short-circuits on it but doesn't stop the later unconditional call.

Test plan

  • Reproduced the panic directly (a relay configured with IndexLongerTag, an event containing a bare single-element tag) before applying the fix, confirmed it no longer panics after.
  • go build, go vet, gofmt -l all clean.

A tag with fewer than 2 elements (e.g. NIP-70's bare ["-"] protected-
event marker) has no tag[1] at all. The len(tag) < 2 check that was
meant to guard this short-circuited the rest of that compound OR
condition correctly, but the branch it fell into then unconditionally
evaluated tag[1] as a call argument to IndexLongerTag -- Go evaluates
call arguments before the call, so this panicked with an out-of-range
index before IndexLongerTag's own body (which does handle a short/empty
value safely) ever ran.

This isn't hypothetical: IndexLongerTag, once configured, is wired for
every event a relay stores, not just ones it authors itself -- so any
client publishing any event containing a bare single-element tag would
crash the whole process.

Fixed by checking len(tag) < 2 on its own, before ever touching tag[1],
rather than folded into the compound condition that (correctly) still
short-circuits on it but doesn't stop the later unconditional call.

go build/go vet/gofmt clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant