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

Panic when token is null for /v1/auth/token/lookup, /v1/auth/token/renew, /v1/auth/token/revoke, /v1/auth/token/revoke-orphan #13225

Closed
ludovicianul opened this issue Nov 19, 2021 · 4 comments
Labels
bug Used to indicate a potential bug core/token

Comments

@ludovicianul
Copy link

Describe the bug
Making a POST at any of the 4 URLs with token = null results in no HTTP response and the following stacktrace:

 invalid syntax"
2021-11-19T14:25:57.498+0200 [INFO]  http: panic serving 127.0.0.1:52114: interface conversion: interface {} is nil, not string
goroutine 227193 [running]:
net/http.(*conn).serve.func1()
        /Users/runner/hostedtoolcache/go/1.17.2/x64/src/net/http/server.go:1801 +0xb9
panic({0x50a5d80, 0xc002395590})
        /Users/runner/hostedtoolcache/go/1.17.2/x64/src/runtime/panic.go:1047 +0x266
github.com/hashicorp/vault/vault.(*Core).handleCancelableRequest(0xc00084cc00, {0x6a43270, 0xc002395110}, 0xc0006ae180)
        /Users/runner/work/vault/vault/vault/request_handling.go:546 +0x1d94
github.com/hashicorp/vault/vault.(*Core).switchedLockHandleRequest(0xc00084cc00, {0x6a43270, 0xc002394e70}, 0xc0006ae180, 0x0)
        /Users/runner/work/vault/vault/vault/request_handling.go:442 +0x4a5
github.com/hashicorp/vault/vault.(*Core).HandleRequest(...)
        /Users/runner/work/vault/vault/vault/request_handling.go:408
github.com/hashicorp/vault/http.request(0x54f6a00, {0x69f4e70, 0xc0014a4b80}, 0xc00134a500, 0xc0006ae180)
        /Users/runner/work/vault/vault/http/handler.go:953 +0x86
github.com/hashicorp/vault/http.handleLogicalInternal.func1({0x69f4e70, 0xc0014a4b80}, 0xc00134a500)
        /Users/runner/work/vault/vault/http/logical.go:341 +0xb6
net/http.HandlerFunc.ServeHTTP(0xc001a5bd15, {0x69f4e70, 0xc0014a4b80}, 0xc001172000)
        /Users/runner/hostedtoolcache/go/1.17.2/x64/src/net/http/server.go:2046 +0x2f
github.com/hashicorp/vault/http.handleRequestForwarding.func1({0x69f4e70, 0xc0014a4b80}, 0xc00134a500)
        /Users/runner/work/vault/vault/http/handler.go:878 +0x39d

To Reproduce
Steps to reproduce the behavior:

  1. Run curl -X POST -H 'Content-Type: application/json' -H "X-Vault-Token: $token" -d '{"token": null}' http://localhost:8200/v1/auth/token/lookup

Same steps to reproduce for all paths.

This was discovered while running a fuzzing tool I wrote for OpenAPI specs: https://github.com/Endava/cats. You can replay all the tests using:
./cats.jar replay --testCases="Test386.json,Test1618.json,Test3540.json,Test4288.json"

TestCases.zip

Expected behavior
A proper HTTP response: 400 with some errors details.

Environment:

  • Vault Server Version (retrieve with vault status): 1.9.0
  • Vault CLI Version (retrieve with vault version): Vault v1.9.0
  • Server Operating System/Architecture: macOS Monterey 12.0.1
@heatherezell heatherezell added core/token bug Used to indicate a potential bug labels Nov 19, 2021
@heatherezell
Copy link
Contributor

Hi @ludovicianul! Thanks for this report, interesting stuff! I'll get some engineers on it next week (most are out today). Thanks again!

@ryowright
Copy link
Contributor

Hi, I've taken a look at this issue and was able to reproduce and fix it on my local environment.

According to the stack trace the error/panic occurs in the handleCancelableRequest function, which is defined in the vault/vault/request_handling.go file. What happens is that any data passed into the request body through a cURL request is stored in req.Data which is a map of strings to interfaces. Because the JSON body is parsed into Golang, when a null value is parsed, it is read in as nil. A variable called token is then assigned this nil interface. The panic occurs when the function attempts to type assert/cast token (which is a nil interface) to a string.

According to a Stack Overflow thread and Golang documentation:

"x.(T) asserts that x is not nil and that the value stored in x is of type T"

I was able to fix this error by adding a condition that checks if the interface is nil before doing any type assertions and it now returns a proper error response.

Response:
Screen Shot 2021-11-20 at 12 29 30 AM

No panic/stack trace on server side:
Screen Shot 2021-11-20 at 12 29 37 AM

@ryowright
Copy link
Contributor

Would I be able to submit a PR for this @hsimon-hashicorp @ludovicianul?

@pmmukh
Copy link
Contributor

pmmukh commented Dec 21, 2021

Fixed in #13236 , with an update to the error message added in #13233

@pmmukh pmmukh closed this as completed Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug core/token
Projects
None yet
Development

No branches or pull requests

4 participants