Skip to content

Commit daeabd2

Browse files
wallyqsneilalexander
authored andcommitted
Fix int32 overflow of JWT account and user limits
JWT claims use int64 for limits (Conn, LeafNodeConn, Subs, Payload) but the server stores them as int32. Direct int32() casts silently truncate values exceeding MaxInt32, causing: - Panic in updateRemoteServer due to slice bounds out of range when mleafs overflows to negative, producing negative slice index - All subscriptions rejected when msubs overflows to negative, making subsAtLimit() always true - All publishes rejected and connection closed when mpay overflows to negative, making payload check always trigger Clamp int64 values to MaxInt32 in updateAccountClaims (account-level) and applyAccountLimits (user-level). Signed-off-by: Waldemar Quevedo <wally@nats.io>
1 parent ec52c6a commit daeabd2

File tree

3 files changed

+191
-8
lines changed

3 files changed

+191
-8
lines changed

server/accounts.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,12 @@ type sconns struct {
138138
leafs int32
139139
}
140140

141+
// clampInt64ToInt32 safely converts an int64 limit to int32,
142+
// clamping values to the [math.MinInt32, math.MaxInt32] range.
143+
func clampInt64ToInt32(v int64) int32 {
144+
return int32(max(math.MinInt32, min(math.MaxInt32, v)))
145+
}
146+
141147
// Import stream mapping struct
142148
type streamImport struct {
143149
acc *Account
@@ -3716,10 +3722,10 @@ func (s *Server) updateAccountClaimsWithRefresh(a *Account, ac *jwt.AccountClaim
37163722

37173723
// Now do limits if they are present.
37183724
a.mu.Lock()
3719-
a.msubs = int32(ac.Limits.Subs)
3720-
a.mpay = int32(ac.Limits.Payload)
3721-
a.mconns = int32(ac.Limits.Conn)
3722-
a.mleafs = int32(ac.Limits.LeafNodeConn)
3725+
a.msubs = clampInt64ToInt32(ac.Limits.Subs)
3726+
a.mpay = clampInt64ToInt32(ac.Limits.Payload)
3727+
a.mconns = clampInt64ToInt32(ac.Limits.Conn)
3728+
a.mleafs = clampInt64ToInt32(ac.Limits.LeafNodeConn)
37233729
a.disallowBearer = ac.Limits.DisallowBearer
37243730
// Check for any revocations
37253731
if len(ac.Revocations) > 0 {

server/client.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -928,14 +928,14 @@ func (c *client) applyAccountLimits() {
928928
c.msubs = jwt.NoLimit
929929
if c.opts.JWT != _EMPTY_ { // user jwt implies account
930930
if uc, _ := jwt.DecodeUserClaims(c.opts.JWT); uc != nil {
931-
atomic.StoreInt32(&c.mpay, int32(uc.Limits.Payload))
932-
c.msubs = int32(uc.Limits.Subs)
931+
atomic.StoreInt32(&c.mpay, clampInt64ToInt32(uc.Limits.Payload))
932+
c.msubs = clampInt64ToInt32(uc.Limits.Subs)
933933
if uc.IssuerAccount != _EMPTY_ && uc.IssuerAccount != uc.Issuer {
934934
if scope, ok := c.acc.signingKeys[uc.Issuer]; ok {
935935
if userScope, ok := scope.(*jwt.UserScope); ok {
936936
// if signing key disappeared or changed and we don't get here, the client will be disconnected
937-
c.mpay = int32(userScope.Template.Limits.Payload)
938-
c.msubs = int32(userScope.Template.Limits.Subs)
937+
c.mpay = clampInt64ToInt32(userScope.Template.Limits.Payload)
938+
c.msubs = clampInt64ToInt32(userScope.Template.Limits.Subs)
939939
}
940940
}
941941
}

server/jwt_test.go

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"errors"
2222
"fmt"
2323
"io"
24+
"math"
2425
"net/http"
2526
"net/http/httptest"
2627
"os"
@@ -7485,3 +7486,179 @@ func TestJWTClusterUserInfoContainsPermissions(t *testing.T) {
74857486
test()
74867487
}
74877488
}
7489+
7490+
func TestJWTAccountLimitsOverflowInt32(t *testing.T) {
7491+
// Without clamping, int32 truncation of values > MaxInt32 causes:
7492+
// - mleafs/mconns wrapping to negative, triggering panics in
7493+
// updateRemoteServer (slice bounds out of range) and rejecting
7494+
// all connections.
7495+
fooAC := newJWTTestAccountClaims()
7496+
fooAC.Limits.Conn = math.MaxInt32 + 1
7497+
fooAC.Limits.LeafNodeConn = math.MaxInt32 + 1
7498+
fooAC.Limits.Subs = math.MaxInt32 + 1
7499+
fooAC.Limits.Payload = math.MaxInt32 + 1
7500+
7501+
s, fooKP, c, _ := setupJWTTestWitAccountClaims(t, fooAC, "+OK")
7502+
defer s.Shutdown()
7503+
defer c.close()
7504+
7505+
fooPub, _ := fooKP.PublicKey()
7506+
fooAcc, _ := s.LookupAccount(fooPub)
7507+
fooAcc.mu.RLock()
7508+
mconns := fooAcc.mconns
7509+
mleafs := fooAcc.mleafs
7510+
msubs := fooAcc.msubs
7511+
mpay := fooAcc.mpay
7512+
fooAcc.mu.RUnlock()
7513+
7514+
// All account limits should be clamped to math.MaxInt32.
7515+
if mconns != math.MaxInt32 {
7516+
t.Fatalf("Expected account mconns to be MaxInt32 (%d), got %d", math.MaxInt32, mconns)
7517+
}
7518+
if mleafs != math.MaxInt32 {
7519+
t.Fatalf("Expected account mleafs to be MaxInt32 (%d), got %d", math.MaxInt32, mleafs)
7520+
}
7521+
if msubs != math.MaxInt32 {
7522+
t.Fatalf("Expected account msubs to be MaxInt32 (%d), got %d", math.MaxInt32, msubs)
7523+
}
7524+
if mpay != math.MaxInt32 {
7525+
t.Fatalf("Expected account mpay to be MaxInt32 (%d), got %d", math.MaxInt32, mpay)
7526+
}
7527+
7528+
// Simulate a remote server update — without clamping this panics with:
7529+
// panic: runtime error: slice bounds out of range [2147483648:0]
7530+
clients := fooAcc.updateRemoteServer(&AccountNumConns{
7531+
Server: ServerInfo{
7532+
ID: "fake-server-1",
7533+
Name: "fake-nats-1",
7534+
},
7535+
AccountStat: AccountStat{
7536+
Account: fooPub,
7537+
Conns: 1,
7538+
LeafNodes: 1,
7539+
},
7540+
})
7541+
if len(clients) != 0 {
7542+
t.Fatalf("Expected no clients to disconnect, got %d", len(clients))
7543+
}
7544+
}
7545+
7546+
func TestJWTUserLimitsOverflowInt32SubPub(t *testing.T) {
7547+
t.Run("Subs", func(t *testing.T) {
7548+
nuc := newJWTTestUserClaims()
7549+
// Without clamping, int32(math.MaxInt32+1) wraps to MinInt32,
7550+
// making subsAtLimit() always true and blocking all subscriptions.
7551+
nuc.Limits.Subs = math.MaxInt32 + 1
7552+
s, c, cr := setupJWTTestWithUserClaims(t, nuc, "+OK")
7553+
defer s.Shutdown()
7554+
defer c.close()
7555+
7556+
expectPong(t, cr)
7557+
7558+
// With clamping, subscriptions should succeed.
7559+
// Before, this would have been `-ERR 'maximum subscriptions exceeded`
7560+
c.parseAsync("SUB foo 1\r\nPING\r\n")
7561+
l, _ := cr.ReadString('\n')
7562+
if !strings.HasPrefix(l, "+OK") {
7563+
t.Fatalf("Expected +OK, got %q", l)
7564+
}
7565+
})
7566+
7567+
t.Run("Payload", func(t *testing.T) {
7568+
nuc := newJWTTestUserClaims()
7569+
// Without clamping, int32(math.MaxInt32+1) wraps to MinInt32,
7570+
// making int64(size) > int64(negative) always true and rejecting
7571+
// all publishes and closing the connection.
7572+
nuc.Limits.Payload = math.MaxInt32 + 1
7573+
s, c, cr := setupJWTTestWithUserClaims(t, nuc, "+OK")
7574+
defer s.Shutdown()
7575+
defer c.close()
7576+
7577+
expectPong(t, cr)
7578+
7579+
// With clamping, publish should succeed.
7580+
// Before, this would have caused `-ERR 'Maximum Payload Violation'`
7581+
// then disconnect the client.
7582+
c.parseAsync("PUB baz 5\r\nhello\r\nPING\r\n")
7583+
l, _ := cr.ReadString('\n')
7584+
if !strings.HasPrefix(l, "+OK") {
7585+
t.Fatalf("Expected +OK, got %q", l)
7586+
}
7587+
})
7588+
7589+
t.Run("ScopedSigningKey", func(t *testing.T) {
7590+
// Without clamping in the scoped signing key path (client.go
7591+
// userScope.Template.Limits), int32 truncation of values >
7592+
// MaxInt32 would wrap to negative and reject all subs/publishes.
7593+
akp, _ := nkeys.CreateAccount()
7594+
apub, _ := akp.PublicKey()
7595+
nac := jwt.NewAccountClaims(apub)
7596+
7597+
// Create a scoped signing key with overflow limits.
7598+
skp, _ := nkeys.CreateAccount()
7599+
spub, _ := skp.PublicKey()
7600+
scope := jwt.NewUserScope()
7601+
scope.Key = spub
7602+
scope.Template.Limits.Subs = math.MaxInt32 + 1
7603+
scope.Template.Limits.Payload = math.MaxInt32 + 1
7604+
nac.SigningKeys.AddScopedSigner(scope)
7605+
7606+
ajwt, err := nac.Encode(oKp)
7607+
require_NoError(t, err)
7608+
7609+
// Create user signed by the scoped signing key.
7610+
// SetScoped(true) clears UserPermissionLimits so the
7611+
// scope template is used instead of user claims.
7612+
ukp, _ := nkeys.CreateUser()
7613+
upub, _ := ukp.PublicKey()
7614+
nuc := jwt.NewUserClaims(upub)
7615+
nuc.IssuerAccount = apub
7616+
nuc.SetScoped(true)
7617+
ujwt, err := nuc.Encode(skp)
7618+
require_NoError(t, err)
7619+
7620+
s := opTrustBasicSetup()
7621+
defer s.Shutdown()
7622+
buildMemAccResolver(s)
7623+
addAccountToMemResolver(s, apub, ajwt)
7624+
7625+
c, cr, l := newClientForServer(s)
7626+
defer c.close()
7627+
7628+
var info nonceInfo
7629+
json.Unmarshal([]byte(l[5:]), &info)
7630+
sigraw, _ := ukp.Sign([]byte(info.Nonce))
7631+
sig := base64.RawURLEncoding.EncodeToString(sigraw)
7632+
7633+
cs := fmt.Sprintf("CONNECT {\"jwt\":%q,\"sig\":\"%s\",\"verbose\":true,\"pedantic\":true}\r\nPING\r\n", ujwt, sig)
7634+
wg := sync.WaitGroup{}
7635+
wg.Add(1)
7636+
go func() {
7637+
c.parse([]byte(cs))
7638+
wg.Done()
7639+
}()
7640+
l, _ = cr.ReadString('\n')
7641+
if !strings.HasPrefix(l, "+OK") {
7642+
t.Fatalf("Expected +OK on CONNECT, got %q", l)
7643+
}
7644+
wg.Wait()
7645+
expectPong(t, cr)
7646+
7647+
// SUB must succeed — without clamping, msubs overflows to
7648+
// negative and subsAtLimit() always returns true.
7649+
c.parseAsync("SUB foo 1\r\nPING\r\n")
7650+
l, _ = cr.ReadString('\n')
7651+
if !strings.HasPrefix(l, "+OK") {
7652+
t.Fatalf("Expected +OK on SUB, got %q", l)
7653+
}
7654+
expectPong(t, cr)
7655+
7656+
// PUB must succeed — without clamping, mpay overflows to
7657+
// negative and the payload check always triggers.
7658+
c.parseAsync("PUB baz 5\r\nhello\r\nPING\r\n")
7659+
l, _ = cr.ReadString('\n')
7660+
if !strings.HasPrefix(l, "+OK") {
7661+
t.Fatalf("Expected +OK on PUB, got %q", l)
7662+
}
7663+
})
7664+
}

0 commit comments

Comments
 (0)