The jwt format is incorrect and panic directly

[liyisker]

err = verifier.Verify(token.Payload(), token.Signature())

Once the JWT format of the previous Token input is incorrect, this code will immediately panic
Go1.17

Error message:
`panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x10 pc=0xbb6c0a]

goroutine 1 [running]:
github.com/cristalhq/jwt/v3.(*Token).Payload(...)
C:/Users/SKER/go/pkg/mod/github.com/cristalhq/jwt/v3@v3.1.0/jwt.go:52
ncovGin/tools.VerifyToken({0xe2f985, 0xc000046000})
C:/go-src/ncovGin/tools/jwt.go:63 +0x6a
main.main()
C:/go-src/ncovGin/test/test.go:14 +0x25
`

[cristaloleg]

Hi, sorry for the trouble, care to show a bit more code? Like how do you create token, verifier. Thanks.

[liyisker]

Ok, the complete code is in the attachment
jwt.txt

[liyisker]

Test use _ ,err =jwt.ParseAndVerifyString(tokenstr,verifier) if err != nil { return err }
input error JWT format, no panic

[cristaloleg]

Hm, have added test:

func TestTest(t *testing.T) {
	c := Claims{
		User: "me",
		StandardClaims: jwt.StandardClaims{
			ID:        "id",
			Audience:  []string{"aud"},
			Issuer:    "me",
			Subject:   "sub",
			ExpiresAt: jwt.NewNumericDate(time.Now()),
			IssuedAt:  jwt.NewNumericDate(time.Now()),
			NotBefore: jwt.NewNumericDate(time.Now()),
		},
	}
	s, err := NewToken(c)
	if err != nil {
		t.Fatal(err)
	}
	t.Log(s)

	cc, err := ParseToken(s)
	if err != nil {
		t.Fatal(err)
	}
	t.Log(cc)

	if err := VerifyToken(s); err != nil {
		t.Fatal(err)
	}
}

And everything looks fine. I've found few things in your code:

1. no error checks in VerifyToken
2. here you're deleting all std claims claims.StandardClaims = jwt.StandardClaims{}

Can you show really FULL code, or an example token which you're trying to parse/verify.

Stacktrace from the original comment says that token has nil fields, which can be achieved only with an ignored error from ParseString here.

[cristaloleg]

Have added

_, err = jwt.ParseAndVerifyString(tokenstr, verifier)
if err != nil {
return err
}

in VerifyToken func and still no error/panic in my test func.

[liyisker]

You may not understand what I mean. My problem is that when the token is incomplete, it does not return an error, but directly panic. A complete token is:

"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJVc2VyIjoibWUiLCJTdGFuZGFyZENsYWltcyI6eyJqdGkiOiJpZCIsImF1ZCI6ImF1ZCIsImlzcyI6Im1lIiwic3ViIjoic3ViIiwiZXhwIjoxNjI5NjQ3NzkxLCJpYXQiOjE2Mjk2NDc3OTEsIm5iZiI6MTYyOTY0Nzc5MX19.-FmGSAZ92EwRSMdWrUdlvwmY3npF5_ISydsyjpsrQnU"

Delete one of his letters:

"YJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJVc2VyIjoibWUiLCJTdGFuZGFyZENsYWltcyI6eyJqdGkiOiJpZCIsImF1ZCI6ImF1ZCIsImlzcyI6Im1lIiwic3ViIjoic3ViIiwiZXhwIjoxNjI5NjQ3NzkxLCJpYXQiOjE2Mjk2NDc3OTEsIm5iZiI6MTYyOTY0Nzc5MX19.-FmGSAZ92EwRSMdWrUdlvwmY3npF5_ISydsyjpsrQnU"

Calling VerifyToken will panic directly instead of returning an error

[cristaloleg]

Ok, got it. The problem is when you pass nil-token (which happens in your original example, due to skipped error checking).

But agree, should not panic. Dumb fix - add note in the docs to not pass nil-token, but better to add an if for every verifier. Will add this PR in 30 mins. Thanks for the suggestion 😉

[cristaloleg]

Hm...not so fast! From the stacktrace: github.com/cristalhq/jwt/v3.(*Token).Payload(...) problem is NOT in Verifier but in incorrectly created Token. That's completely another problem.

[liyisker]

This method can bypass this problem, and the incorrectly entered Token will no longer be panic.

func VerifyToken(tokenstr string) error {
	// and verify it's signature
	verifier, err := jwt.NewVerifierHS(jwt.HS256, key)

	_ ,err =jwt.ParseAndVerifyString(tokenstr,verifier)
	if err != nil {
		return err
	}
	return nil
}

[cristaloleg]

Looks like it cannot bypass. parse cannot return nil-token and nil-error at once.

jwt/parse.go

Lines 19 to 37 in bf8716c

	// ParseAndVerifyString decodes a token and verifies it's signature.
	func ParseAndVerifyString(raw string, verifier Verifier) (*Token, error) {
	return ParseAndVerify([]byte(raw), verifier)
	}
	// ParseAndVerify decodes a token and verifies it's signature.
	func ParseAndVerify(raw []byte, verifier Verifier) (*Token, error) {
	token, err := parse(raw)
	if err != nil {
	return nil, err
	}
	if !constTimeAlgEqual(token.Header().Algorithm, verifier.Algorithm()) {
	return nil, ErrAlgorithmMismatch
	}
	if err := verifier.Verify(token.Payload(), token.Signature()); err != nil {
	return nil, err
	}
	return token, nil
	}

So, here only 1 returned variable can be nil

jwt/parse.go

Line 26 in bf8716c

token, err := parse(raw)

if it's non-nil error - we return from the func, if it's nil - token isn't nil and token.Payload() will not fail.

[cristaloleg]

This do not panic as for bf8716c

func TestTest(t *testing.T) {
	token := "bad-token-whatever"
	if err := VerifyToken(token); err != nil {
		t.Fatal(err)
	}
}

func VerifyToken(tokenstr string) error {
	// and verify it's signature
	verifier, err := jwt.NewVerifierHS(jwt.HS256, key)

	_, err = jwt.ParseAndVerifyString(tokenstr, verifier)
	if err != nil {
		return err
	}
	return nil
}

[cristaloleg]

Hi @liyisker can you clarify that we've discussed everything and this can be closed? or we've something to discuss? Thank you.

[cristaloleg]

Closing for now, feel free to reopen.