Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic caused by parsing json.Number values for TypeCommaStringSlice fields #14522

Merged
merged 6 commits into from Mar 28, 2022

Conversation

ccapurso
Copy link
Contributor

@ccapurso ccapurso commented Mar 16, 2022

Vault will panic when it attempts to parse fields with a schema of TypeCommaStringSlice if the provided value is an integer specifically if provided within the JSON body of an HTTP request. The underlying cause is within the ParseCommaStringSlice function in go-secure-stdlib/parseutil. The issue has been fixed in go-secure-stdlib #29.

The changes introduced within this PR are:

  • Upgrading to v0.1.4 of go-secure-stdlib/parseutil
  • Adding a test specifically for json.Number input

Before fix:

❯ vault auth enable userpass
Success! Enabled userpass auth method at: userpass/

❯ curl -v -XPOST -H "X-Vault-Token: $VAULT_TOKEN" --data '{"password":"abc123", "token_bound_cidrs": 123}' "$VAULT_ADDR/v1/auth/userpass/users/jdoe"
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8200 (#0)
> POST /v1/auth/userpass/users/jdoe HTTP/1.1
> Host: 127.0.0.1:8200
> User-Agent: curl/7.64.1
> Accept: */*
> X-Vault-Token: hvs.ogFGaKQ3F4DUEh2OtQ39L1bf
> Content-Length: 47
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 47 out of 47 bytes
* Empty reply from server
* Connection #0 to host 127.0.0.1 left intact
curl: (52) Empty reply from server
* Closing connection 0

Panic in server logs (truncated):

2022-03-16T10:42:16.943-0400 [INFO]  http: panic serving 127.0.0.1:51571: interface conversion: interface {} is json.Number, not string
goroutine 854 [running]:
net/http.(*conn).serve.func1()
        /usr/local/bin/go/src/net/http/server.go:1802 +0xb9
panic({0x514d440, 0xc000cce810})
        /usr/local/bin/go/src/runtime/panic.go:1047 +0x266
github.com/mitchellh/mapstructure.StringToSliceHookFunc.func1(0x51a9ec0, 0xc000dfd720, {0x51a9ec0, 0xc000dfd720})
        /Users/ccapurso/go/pkg/mod/github.com/mitchellh/mapstructure@v1.4.3/decode_hooks.go:91 +0xd1
github.com/mitchellh/mapstructure.DecodeHookExec({0x50be8e0, 0xc0009cbbc0}, {0x51a9ec0, 0xc000dfd720, 0x1054e26}, {0x4cf4fe0, 0xc0009cbba8, 0x203000})
        /Users/ccapurso/go/pkg/mod/github.com/mitchellh/mapstructure@v1.4.3/decode_hooks.go:49 +0x1f9
github.com/mitchellh/mapstructure.(*Decoder).decode(0xc001482120, {0x0, 0xc000cca501}, {0x51a9ec0, 0xc000dfd720}, {0x4cf4fe0, 0xc0009cbba8, 0x48})
        /Users/ccapurso/go/pkg/mod/github.com/mitchellh/mapstructure@v1.4.3/mapstructure.go:440 +0x179
github.com/mitchellh/mapstructure.(*Decoder).Decode(0xc001482120, {0x51a9ec0, 0xc000dfd720})
        /Users/ccapurso/go/pkg/mod/github.com/mitchellh/mapstructure@v1.4.3/mapstructure.go:398 +0xd8
github.com/hashicorp/go-secure-stdlib/parseutil.ParseCommaStringSlice({0x51a9ec0, 0xc000dfd720})
        /Users/ccapurso/go/pkg/mod/github.com/hashicorp/go-secure-stdlib/parseutil@v0.1.3/parseutil.go:354 +0x11e
github.com/hashicorp/vault/sdk/framework.(*FieldData).getPrimitive(0x50594e0, {0xc001325788, 0xc001325788}, 0xc0014a2230)
        /Users/ccapurso/git/vault/sdk/framework/field_data.go:289 +0x71b
github.com/hashicorp/vault/sdk/framework.(*FieldData).Validate(0xc000dfd8c0)
        /Users/ccapurso/git/vault/sdk/framework/field_data.go:44 +0xf3
github.com/hashicorp/vault/sdk/framework.(*Backend).HandleExistenceCheck(0x0, {0x64dd010, 0xc000cce4e0}, 0xc0001e9e00)
        /Users/ccapurso/git/vault/sdk/framework/backend.go:174 +0x3b4
github.com/hashicorp/vault/builtin/plugin.(*PluginBackend).HandleExistenceCheck.func1()
        /Users/ccapurso/git/vault/builtin/plugin/backend.go:211 +0x43
github.com/hashicorp/vault/builtin/plugin.(*PluginBackend).lazyLoadBackend(0xc001081860, {0x64dd010, 0xc000cce4e0}, {0x64de8c8, 0xc0007b7780}, 0xc000fe01d0)
        /Users/ccapurso/git/vault/builtin/plugin/backend.go:162 +0x19d
github.com/hashicorp/vault/builtin/plugin.(*PluginBackend).HandleExistenceCheck(0xc0007e1ea0, {0x64dd010, 0xc000cce4e0}, 0xc000e2dcb9)
        /Users/ccapurso/git/vault/builtin/plugin/backend.go:209 +0x9f
github.com/hashicorp/vault/vault.(*Router).routeCommon(0xc00047f9f0, {0x64dd010, 0xc000cce4e0}, 0xc0001e9e00, 0x1)
        /Users/ccapurso/git/vault/vault/router.go:720 +0x18a9
github.com/hashicorp/vault/vault.(*Router).RouteExistenceCheck(0x505c120, {0x64dd010, 0xc000cce4e0}, 0xd)
        /Users/ccapurso/git/vault/vault/router.go:511 +0x28
github.com/hashicorp/vault/vault.(*Core).checkToken(0xc00089ea00, {0x64dd010, 0xc000cce4e0}, 0xc0001e9e00, 0x0)
        /Users/ccapurso/git/vault/vault/request_handling.go:318 +0x6ab

After fix:

❯ vault auth enable userpass
Success! Enabled userpass auth method at: userpass/

❯ curl -v -XPOST -H "X-Vault-Token: $VAULT_TOKEN" --data '{"password":"abc123", "token_bound_cidrs": 123}' "$VAULT_ADDR/v1/auth/userpass/users/jdoe"
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8200 (#0)
> POST /v1/auth/userpass/users/jdoe HTTP/1.1
> Host: 127.0.0.1:8200
> User-Agent: curl/7.64.1
> Accept: */*
> X-Vault-Token: hvs.8isydRBWneASw7nO8wYfISCt
> Content-Length: 47
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 47 out of 47 bytes
< HTTP/1.1 400 Bad Request
< Cache-Control: no-store
< Content-Type: application/json
< Strict-Transport-Security: max-age=31536000; includeSubDomains
< Date: Wed, 16 Mar 2022 14:44:29 GMT
< Content-Length: 117
<
{"errors":["error parsing address \"123\": Unable to convert \"123\" to an IPv4 or IPv6 address, or a UNIX Socket"]}
* Connection #0 to host 127.0.0.1 left intact
* Closing connection 0

@vercel vercel bot temporarily deployed to Preview – vault March 16, 2022 14:01 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook March 16, 2022 14:01 Inactive
@ccapurso ccapurso requested a review from a team March 16, 2022 14:01
@vercel vercel bot temporarily deployed to Preview – vault-storybook March 16, 2022 14:33 Inactive
@vercel vercel bot temporarily deployed to Preview – vault March 16, 2022 14:33 Inactive
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.

None yet

2 participants