Skip to content

Commit 1c87b21

Browse files
authored
fix: guard against nil panics in JWT and watermark paths (#787)
fix: guard against nil dereference in GenerateToken and watermark rejection Move the ok check before any use of ud in JWT.GenerateToken to prevent a nil dereference when GetUserData returns (nil, false). Also guard the bare type assertion on msg.(request.WithWatermark) in the watermark rejection metrics path, matching the safe pattern used a few lines below.
1 parent 3f51d46 commit 1c87b21

3 files changed

Lines changed: 28 additions & 8 deletions

File tree

pkg/middlewares/jwt.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -149,17 +149,19 @@ func (j *JWT) GenerateToken(user string, pass string) (string, error) {
149149
claims := token.Claims.(jwt.MapClaims)
150150
claims["user"] = user
151151
ud, ok := j.GetUserData(user)
152+
if !ok {
153+
return "", fmt.Errorf("JWT: user not found")
154+
}
152155
if !constantTimeCompare(pass, ud.Password) {
153156
ud = ud.NewData
154157
}
155-
if ok {
156-
if ud.Exp == 0 {
157-
ud.Exp = 60
158-
}
159-
claims["exp"] = time.Now().Add(time.Minute * time.Duration(ud.Exp)).Unix()
160-
} else {
161-
return "", fmt.Errorf("JWT: user not found")
158+
if ud == nil {
159+
return "", fmt.Errorf("JWT: invalid credentials")
160+
}
161+
if ud.Exp == 0 {
162+
ud.Exp = 60
162163
}
164+
claims["exp"] = time.Now().Add(time.Minute * time.Duration(ud.Exp)).Unix()
163165
token.Claims = claims
164166
return token.SignedString([]byte(ud.Secret))
165167
}

pkg/middlewares/jwt_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,20 @@ func TestAuthenticateWithCredentialRotation(t *testing.T) {
435435
})
436436
}
437437

438+
func TestGenerateTokenNonexistentUser(t *testing.T) {
439+
j := &JWT{
440+
Users: map[string]UserData{
441+
"user": {
442+
Password: "pass",
443+
Secret: secret,
444+
},
445+
},
446+
}
447+
_, err := j.GenerateToken("ghost", "pass")
448+
require.Error(t, err)
449+
require.Contains(t, err.Error(), "user not found")
450+
}
451+
438452
func getToken(user string, au *JWTMiddleware) (string, error) {
439453

440454
req, err := http.NewRequest("GET", "/login", nil)

pkg/signatory/signatory.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,11 @@ func (s *Signatory) Sign(ctx context.Context, req *SignRequest) (crypt.Signature
406406

407407
if wmErr != nil {
408408
err = errors.Wrap(wmErr, http.StatusConflict)
409-
metrics.WatermarkRejection(req.PublicKeyHash.String(), msg.SignRequestKind(), msg.(request.WithWatermark).GetChainID().String(), err.Error())
409+
chainIDStr := ""
410+
if m, ok := msg.(request.WithWatermark); ok {
411+
chainIDStr = m.GetChainID().String()
412+
}
413+
metrics.WatermarkRejection(req.PublicKeyHash.String(), msg.SignRequestKind(), chainIDStr, wmErr.Error())
410414
l.Error(err)
411415
return nil, err
412416
}

0 commit comments

Comments
 (0)