Skip to content

Fix nil pointer dereference for interface-typed flag values - #2400

Open
mrueg wants to merge 1 commit into
urfave:mainfrom
mrueg:fix/nil-deref-interface-typed-flags
Open

Fix nil pointer dereference for interface-typed flag values#2400
mrueg wants to merge 1 commit into
urfave:mainfrom
mrueg:fix/nil-deref-interface-typed-flags

Conversation

@mrueg

@mrueg mrueg commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

reflect.TypeOf returns nil when passed a nil interface value. FlagBase is generic over its value type T, and for GenericFlag that type is the Value interface, so both reflect.TypeOf(f.Value) and reflect.TypeOf(zero) yield nil there. Two call sites dereference the result without checking, panicking on an otherwise perfectly valid flag.

SchemaItemsType()

Panics for any GenericFlag, since it unconditionally calls reflect.TypeOf(zero).Kind():

fl := &cli.GenericFlag{Name: "gen"}
fl.SchemaItemsType() // panic: runtime error: invalid memory address or nil pointer dereference

PostParse()

Panics for a GenericFlag whose value source resolves to an empty string:

os.Setenv("SOME_VAR", "")
fl := &cli.GenericFlag{Name: "gen", Sources: cli.EnvVars("SOME_VAR")}
fl.PreParse()
fl.PostParse() // panic: runtime error: invalid memory address or nil pointer dereference

The val != "" short circuit means reflect.TypeOf(f.Value).Kind() is never evaluated when the source produces a non-empty value, so this only reproduces with an empty environment variable or an empty file. That is why it has gone unnoticed.

Fix

TypeName() and IsMultiValueFlag() already guard against a nil type; this does the same in both of the unguarded spots. PostParse() now derives the kind once up front and reuses it, which also removes a duplicate reflect.TypeOf call.

No public API change — go doc -all output is identical to testdata/godoc-v3.x.txt.

Testing

Adds TestFlagBaseInterfaceValueType covering SchemaItemsType, PostParse with an empty source value, and PostParse with a non-empty source value (to pin the non-panicking path). Both subtests panic on main without the change.

🤖 Generated with Claude Code

reflect.TypeOf returns nil when passed a nil interface value. FlagBase
is generic over the value type T, and for GenericFlag that type is the
Value interface, so reflect.TypeOf(f.Value) and reflect.TypeOf(zero)
both yield nil there. Two call sites dereferenced the result without
checking, panicking on an otherwise valid flag:

  * SchemaItemsType() panics for any GenericFlag, since it always
    inspects reflect.TypeOf(zero).Kind().

  * PostParse() panics for a GenericFlag whose value source resolves to
    an empty string. The `val != ""` short circuit hid this whenever the
    source produced a non-empty value, so it only reproduces with an
    empty environment variable or file.

TypeName() and IsMultiValueFlag() already guard against a nil type; do
the same in both of these. PostParse() now derives the kind once and
reuses it, which also drops a duplicate reflect.TypeOf call.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mrueg
mrueg requested a review from a team as a code owner August 10, 2026 15:51
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