purego: validate callback inputs before touching reflect (#512)
NewCallback (unix and windows) called ty.NumIn() before checking the kind, so non-function input died inside reflect (or with a bare nil dereference) instead of the intended purego error. RegisterFunc called Value.Elem() before checking for a function pointer with the same effect. Validate first so misuse fails with actionable messages. Closes #507
- commit
- 9ba4c4002b4ec4da6b3d2585b0b4153ecadc5fde
- author
- Hiroki KUMAZAKI · · 2026-09-12 06:59 UTC
- committer
- GitHub · · 2026-09-12 06:59 UTC
- parent
- 72b0dcd
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
4 files · +65 -1
M func.go +5 -1
| @@ -130,7 +130,11 @@ func RegisterLibFunc(fptr any, handle uintptr, name string) { | |||
| 130 | 130 | // [Cgo rules]: https://pkg.go.dev/cmd/cgo#hdr-Go_references_to_C | |
| 131 | 131 | func RegisterFunc(fptr any, cfn uintptr) { | |
| 132 | 132 | const is32bit = unsafe.Sizeof(uintptr(0)) == 4 | |
| 133 | - | fn := reflect.ValueOf(fptr).Elem() | |
| 133 | + | rv := reflect.ValueOf(fptr) | |
| 134 | + | if rv.Kind() != reflect.Ptr || rv.IsNil() { | |
| 135 | + | panic("purego: fptr must be a non-nil function pointer") | |
| 136 | + | } | |
| 137 | + | fn := rv.Elem() | |
| 134 | 138 | ty := fn.Type() | |
| 135 | 139 | if ty.Kind() != reflect.Func { | |
| 136 | 140 | panic("purego: fptr must be a function pointer") | |
| @@ -130,7 +130,11 @@ func RegisterLibFunc(fptr any, handle uintptr, name string) { | |||||
| 130 | // [Cgo rules]: https://pkg.go.dev/cmd/cgo#hdr-Go_references_to_C | 130 | // [Cgo rules]: https://pkg.go.dev/cmd/cgo#hdr-Go_references_to_C | ||
| 131 | func RegisterFunc(fptr any, cfn uintptr) { | 131 | func RegisterFunc(fptr any, cfn uintptr) { | ||
| 132 | const is32bit = unsafe.Sizeof(uintptr(0)) == 4 | 132 | const is32bit = unsafe.Sizeof(uintptr(0)) == 4 | ||
| 133 | - | fn := reflect.ValueOf(fptr).Elem() | 133 | + | rv := reflect.ValueOf(fptr) |
| 134 | + | if rv.Kind() != reflect.Ptr || rv.IsNil() { | |||
| 135 | + | panic("purego: fptr must be a non-nil function pointer") | |||
| 136 | + | } | |||
| 137 | + | fn := rv.Elem() | |||
| 134 | ty := fn.Type() | 138 | ty := fn.Type() | ||
| 135 | if ty.Kind() != reflect.Func { | 139 | if ty.Kind() != reflect.Func { | ||
| 136 | panic("purego: fptr must be a function pointer") | 140 | panic("purego: fptr must be a function pointer") | ||
M func_test.go +54 -0
| @@ -181,6 +181,60 @@ func TestRegisterLibFunc_Bool(t *testing.T) { | |||
| 181 | 181 | } | |
| 182 | 182 | } | |
| 183 | 183 | ||
| 184 | + | func TestNewCallback_NotAFunction(t *testing.T) { | |
| 185 | + | for _, tc := range []struct { | |
| 186 | + | name string | |
| 187 | + | fn any | |
| 188 | + | }{ | |
| 189 | + | {"nil", nil}, | |
| 190 | + | {"int", 42}, | |
| 191 | + | {"string", "not a function"}, | |
| 192 | + | {"pointer", new(int)}, | |
| 193 | + | } { | |
| 194 | + | t.Run(tc.name, func(t *testing.T) { | |
| 195 | + | defer func() { | |
| 196 | + | r := recover() | |
| 197 | + | if r == nil { | |
| 198 | + | t.Fatal("NewCallback did not panic") | |
| 199 | + | } | |
| 200 | + | const want = "purego: the type must be a function but was not" | |
| 201 | + | if got := fmt.Sprint(r); got != want { | |
| 202 | + | t.Fatalf("panic mismatch:\n got: %q\n want: %q", got, want) | |
| 203 | + | } | |
| 204 | + | }() | |
| 205 | + | purego.NewCallback(tc.fn) | |
| 206 | + | }) | |
| 207 | + | } | |
| 208 | + | } | |
| 209 | + | ||
| 210 | + | func TestRegisterFunc_InvalidFunctionPointer(t *testing.T) { | |
| 211 | + | for _, tc := range []struct { | |
| 212 | + | name string | |
| 213 | + | fptr any | |
| 214 | + | want string | |
| 215 | + | }{ | |
| 216 | + | {"nil", nil, "purego: fptr must be a non-nil function pointer"}, | |
| 217 | + | {"non_pointer", 42, "purego: fptr must be a non-nil function pointer"}, | |
| 218 | + | {"function_value", func() {}, "purego: fptr must be a non-nil function pointer"}, | |
| 219 | + | {"nil_function_pointer", (*func())(nil), "purego: fptr must be a non-nil function pointer"}, | |
| 220 | + | {"pointer_to_non_function", new(int), "purego: fptr must be a function pointer"}, | |
| 221 | + | {"pointer_to_function_pointer", new(*func()), "purego: fptr must be a function pointer"}, | |
| 222 | + | } { | |
| 223 | + | t.Run(tc.name, func(t *testing.T) { | |
| 224 | + | defer func() { | |
| 225 | + | r := recover() | |
| 226 | + | if r == nil { | |
| 227 | + | t.Fatal("RegisterFunc did not panic") | |
| 228 | + | } | |
| 229 | + | if got := fmt.Sprint(r); got != tc.want { | |
| 230 | + | t.Fatalf("panic mismatch:\n got: %q\n want: %q", got, tc.want) | |
| 231 | + | } | |
| 232 | + | }() | |
| 233 | + | purego.RegisterFunc(tc.fptr, 1) | |
| 234 | + | }) | |
| 235 | + | } | |
| 236 | + | } | |
| 237 | + | ||
| 184 | 238 | func TestABI(t *testing.T) { | |
| 185 | 239 | libFileName := filepath.Join(t.TempDir(), "abitest.so") | |
| 186 | 240 | t.Logf("Build %v", libFileName) | |
| @@ -181,6 +181,60 @@ func TestRegisterLibFunc_Bool(t *testing.T) { | |||||
| 181 | } | 181 | } | ||
| 182 | } | 182 | } | ||
| 183 | 183 | ||||
| 184 | + | func TestNewCallback_NotAFunction(t *testing.T) { | |||
| 185 | + | for _, tc := range []struct { | |||
| 186 | + | name string | |||
| 187 | + | fn any | |||
| 188 | + | }{ | |||
| 189 | + | {"nil", nil}, | |||
| 190 | + | {"int", 42}, | |||
| 191 | + | {"string", "not a function"}, | |||
| 192 | + | {"pointer", new(int)}, | |||
| 193 | + | } { | |||
| 194 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 195 | + | defer func() { | |||
| 196 | + | r := recover() | |||
| 197 | + | if r == nil { | |||
| 198 | + | t.Fatal("NewCallback did not panic") | |||
| 199 | + | } | |||
| 200 | + | const want = "purego: the type must be a function but was not" | |||
| 201 | + | if got := fmt.Sprint(r); got != want { | |||
| 202 | + | t.Fatalf("panic mismatch:\n got: %q\n want: %q", got, want) | |||
| 203 | + | } | |||
| 204 | + | }() | |||
| 205 | + | purego.NewCallback(tc.fn) | |||
| 206 | + | }) | |||
| 207 | + | } | |||
| 208 | + | } | |||
| 209 | + | ||||
| 210 | + | func TestRegisterFunc_InvalidFunctionPointer(t *testing.T) { | |||
| 211 | + | for _, tc := range []struct { | |||
| 212 | + | name string | |||
| 213 | + | fptr any | |||
| 214 | + | want string | |||
| 215 | + | }{ | |||
| 216 | + | {"nil", nil, "purego: fptr must be a non-nil function pointer"}, | |||
| 217 | + | {"non_pointer", 42, "purego: fptr must be a non-nil function pointer"}, | |||
| 218 | + | {"function_value", func() {}, "purego: fptr must be a non-nil function pointer"}, | |||
| 219 | + | {"nil_function_pointer", (*func())(nil), "purego: fptr must be a non-nil function pointer"}, | |||
| 220 | + | {"pointer_to_non_function", new(int), "purego: fptr must be a function pointer"}, | |||
| 221 | + | {"pointer_to_function_pointer", new(*func()), "purego: fptr must be a function pointer"}, | |||
| 222 | + | } { | |||
| 223 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 224 | + | defer func() { | |||
| 225 | + | r := recover() | |||
| 226 | + | if r == nil { | |||
| 227 | + | t.Fatal("RegisterFunc did not panic") | |||
| 228 | + | } | |||
| 229 | + | if got := fmt.Sprint(r); got != tc.want { | |||
| 230 | + | t.Fatalf("panic mismatch:\n got: %q\n want: %q", got, tc.want) | |||
| 231 | + | } | |||
| 232 | + | }() | |||
| 233 | + | purego.RegisterFunc(tc.fptr, 1) | |||
| 234 | + | }) | |||
| 235 | + | } | |||
| 236 | + | } | |||
| 237 | + | ||||
| 184 | func TestABI(t *testing.T) { | 238 | func TestABI(t *testing.T) { | ||
| 185 | libFileName := filepath.Join(t.TempDir(), "abitest.so") | 239 | libFileName := filepath.Join(t.TempDir(), "abitest.so") | ||
| 186 | t.Logf("Build %v", libFileName) | 240 | t.Logf("Build %v", libFileName) | ||
M syscall_unix.go +3 -0
| @@ -27,6 +27,9 @@ func syscall_syscallN(fn uintptr, args ...uintptr) (r1, r2, err uintptr) { | |||
| 27 | 27 | // provides similar functionality to windows.NewCallback it is distinct. | |
| 28 | 28 | func NewCallback(fn any) uintptr { | |
| 29 | 29 | ty := reflect.TypeOf(fn) | |
| 30 | + | if ty == nil || ty.Kind() != reflect.Func { | |
| 31 | + | panic("purego: the type must be a function but was not") | |
| 32 | + | } | |
| 30 | 33 | for i := range ty.NumIn() { | |
| 31 | 34 | in := ty.In(i) | |
| 32 | 35 | if !in.AssignableTo(reflect.TypeFor[CDecl]()) { | |
| @@ -27,6 +27,9 @@ func syscall_syscallN(fn uintptr, args ...uintptr) (r1, r2, err uintptr) { | |||||
| 27 | // provides similar functionality to windows.NewCallback it is distinct. | 27 | // provides similar functionality to windows.NewCallback it is distinct. | ||
| 28 | func NewCallback(fn any) uintptr { | 28 | func NewCallback(fn any) uintptr { | ||
| 29 | ty := reflect.TypeOf(fn) | 29 | ty := reflect.TypeOf(fn) | ||
| 30 | + | if ty == nil || ty.Kind() != reflect.Func { | |||
| 31 | + | panic("purego: the type must be a function but was not") | |||
| 32 | + | } | |||
| 30 | for i := range ty.NumIn() { | 33 | for i := range ty.NumIn() { | ||
| 31 | in := ty.In(i) | 34 | in := ty.In(i) | ||
| 32 | if !in.AssignableTo(reflect.TypeFor[CDecl]()) { | 35 | if !in.AssignableTo(reflect.TypeFor[CDecl]()) { | ||
M syscall_windows.go +3 -0
| @@ -26,6 +26,9 @@ func syscall_syscallN(fn uintptr, args ...uintptr) (r1, r2, err uintptr) { | |||
| 26 | 26 | func NewCallback(fn any) uintptr { | |
| 27 | 27 | isCDecl := false | |
| 28 | 28 | ty := reflect.TypeOf(fn) | |
| 29 | + | if ty == nil || ty.Kind() != reflect.Func { | |
| 30 | + | panic("purego: the type must be a function but was not") | |
| 31 | + | } | |
| 29 | 32 | for i := range ty.NumIn() { | |
| 30 | 33 | in := ty.In(i) | |
| 31 | 34 | if !in.AssignableTo(reflect.TypeFor[CDecl]()) { | |
| @@ -26,6 +26,9 @@ func syscall_syscallN(fn uintptr, args ...uintptr) (r1, r2, err uintptr) { | |||||
| 26 | func NewCallback(fn any) uintptr { | 26 | func NewCallback(fn any) uintptr { | ||
| 27 | isCDecl := false | 27 | isCDecl := false | ||
| 28 | ty := reflect.TypeOf(fn) | 28 | ty := reflect.TypeOf(fn) | ||
| 29 | + | if ty == nil || ty.Kind() != reflect.Func { | |||
| 30 | + | panic("purego: the type must be a function but was not") | |||
| 31 | + | } | |||
| 29 | for i := range ty.NumIn() { | 32 | for i := range ty.NumIn() { | ||
| 30 | in := ty.In(i) | 33 | in := ty.In(i) | ||
| 31 | if !in.AssignableTo(reflect.TypeFor[CDecl]()) { | 34 | if !in.AssignableTo(reflect.TypeFor[CDecl]()) { | ||