Size computation for allocation may overflow (#99)

* Size computation for allocation may overflow

Performing calculations involving the size of potentially large strings or slices can result in an overflow (for signed integer types) or a wraparound (for unsigned types). An overflow causes the result of the calculation to become negative, while a wraparound results in a small (positive) number.
This commit is contained in:
2025-12-08 11:22:28 +00:00
committed by GitHub
parent 56051779ee
commit a750c4f5b9
93 changed files with 10500 additions and 443 deletions
+2 -2
View File
@@ -76,7 +76,7 @@ func NewRedisBackend(config *Config) (*RedisBackend, error) {
// Test connectivity
if err := backend.Ping(context.Background()); err != nil {
pool.Close()
_ = pool.Close()
return nil, fmt.Errorf("failed to ping Redis: %w", err)
}
@@ -263,7 +263,7 @@ func (r *RedisBackend) Clear(ctx context.Context) error {
if err != nil {
continue
}
conn.Do("DEL", key) // Best effort, ignore errors
_, _ = conn.Do("DEL", key) // Best effort, ignore errors
}
return nil
+23 -22
View File
@@ -82,7 +82,7 @@ func (p *ConnectionPool) Get(ctx context.Context) (*RedisConn, error) {
// Reuse existing connection - validate if health check enabled
if p.config.EnableHealthCheck && !p.isConnectionHealthy(conn) {
// Connection is stale, close it and try again
conn.Close()
_ = conn.Close()
p.totalConns.Add(-1)
continue
}
@@ -94,6 +94,7 @@ func (p *ConnectionPool) Get(ctx context.Context) (*RedisConn, error) {
default:
// No available connection, create new one if under limit
// #nosec G115 -- MaxConnections is a small config value that fits in int32
if p.totalConns.Load() < int32(p.config.MaxConnections) {
conn, err = p.createConnection()
if err != nil {
@@ -115,7 +116,7 @@ func (p *ConnectionPool) Get(ctx context.Context) (*RedisConn, error) {
case conn = <-p.connections:
// Validate connection if health check enabled
if p.config.EnableHealthCheck && !p.isConnectionHealthy(conn) {
conn.Close()
_ = conn.Close()
p.totalConns.Add(-1)
continue
}
@@ -144,7 +145,7 @@ func (p *ConnectionPool) Put(conn *RedisConn) {
p.activeConns.Add(-1)
if p.closed.Load() || conn.closed.Load() {
conn.Close()
_ = conn.Close()
p.totalConns.Add(-1)
return
}
@@ -155,7 +156,7 @@ func (p *ConnectionPool) Put(conn *RedisConn) {
// Successfully returned to pool
default:
// Pool full, close connection
conn.Close()
_ = conn.Close()
p.totalConns.Add(-1)
}
}
@@ -173,7 +174,7 @@ func (p *ConnectionPool) Close() error {
// Close all pooled connections
for conn := range p.connections {
conn.Close()
_ = conn.Close()
}
return nil
@@ -212,7 +213,7 @@ func (p *ConnectionPool) createConnection() (*RedisConn, error) {
// Authenticate if password is provided
if p.config.Password != "" {
if _, err := redisConn.Do("AUTH", p.config.Password); err != nil {
redisConn.Close()
_ = redisConn.Close()
return nil, fmt.Errorf("authentication failed: %w", err)
}
}
@@ -220,7 +221,7 @@ func (p *ConnectionPool) createConnection() (*RedisConn, error) {
// Select database
if p.config.DB != 0 {
if _, err := redisConn.Do("SELECT", fmt.Sprintf("%d", p.config.DB)); err != nil {
redisConn.Close()
_ = redisConn.Close()
return nil, fmt.Errorf("failed to select database: %w", err)
}
}
@@ -246,15 +247,15 @@ func (c *RedisConn) Do(command string, args ...string) (interface{}, error) {
c.mu.Lock()
defer c.mu.Unlock()
// Build command arguments
// Check for overflow: ensure len(args)+1 doesn't cause allocation overflow
// Limit to a safe value that prevents integer overflow in allocation size calculation
// (capacity * sizeof(string) must fit in int/size_t)
argsLen := len(args)
const maxSafeArgs = (1 << 20) - 1 // 1M args is already absurdly large for Redis commands
if argsLen < 0 || argsLen > maxSafeArgs {
return nil, errors.New("too many arguments")
// Validate argument count to prevent integer overflow in slice operations
// maxSafeArgs is set to (1<<20)-1 = 1,048,575 which is more than any reasonable Redis command
const maxSafeArgs = (1 << 20) - 1
if len(args) > maxSafeArgs {
return nil, errors.New("too many arguments: exceeds maximum safe count")
}
// Build command arguments
// Validate total argument size to prevent memory exhaustion
const maxTotalArgBytes = 64 << 20 // 64 MiB max total size
totalBytes := len(command)
for _, s := range args {
@@ -267,13 +268,13 @@ func (c *RedisConn) Do(command string, args ...string) (interface{}, error) {
return nil, errors.New("total argument size exceeds maximum allowed")
}
}
cmdArgs := make([]string, 0, argsLen+1)
cmdArgs = append(cmdArgs, command)
cmdArgs = append(cmdArgs, args...)
// Build command slice: prepend command to args
// Using append avoids arithmetic on potentially large len(args)
cmdArgs := append([]string{command}, args...)
// Set write timeout
if c.writeTimeout > 0 {
c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout))
_ = c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout))
}
// Write command (using pooled writer for memory efficiency)
@@ -287,7 +288,7 @@ func (c *RedisConn) Do(command string, args ...string) (interface{}, error) {
// Set read timeout
if c.readTimeout > 0 {
c.conn.SetReadDeadline(time.Now().Add(c.readTimeout))
_ = c.conn.SetReadDeadline(time.Now().Add(c.readTimeout))
}
// Read response (using pooled reader for memory efficiency)
@@ -328,8 +329,8 @@ func (p *ConnectionPool) isConnectionHealthy(conn *RedisConn) bool {
// Set a read deadline for the ping
if conn.conn != nil {
conn.conn.SetReadDeadline(time.Now().Add(1 * time.Second))
defer conn.conn.SetReadDeadline(time.Time{}) // Clear deadline
_ = conn.conn.SetReadDeadline(time.Now().Add(1 * time.Second))
defer func() { _ = conn.conn.SetReadDeadline(time.Time{}) }() // Clear deadline
}
_, err := conn.Do("PING")
+3
View File
@@ -158,6 +158,7 @@ func (cb *CircuitBreaker) AllowRequest() bool {
case StateHalfOpen:
// Allow limited requests in half-open state
current := cb.halfOpenRequests.Add(1)
// #nosec G115 -- HalfOpenMaxRequests is a small config value that fits in int32
return current <= int32(cb.config.HalfOpenMaxRequests)
default:
@@ -181,6 +182,7 @@ func (cb *CircuitBreaker) RecordSuccess() {
case StateHalfOpen:
// If we've had enough successful requests, close the circuit
successfulRequests := cb.halfOpenRequests.Load()
// #nosec G115 -- HalfOpenMaxRequests is a small config value that fits in int32
if successfulRequests >= int32(cb.config.HalfOpenMaxRequests) {
cb.setState(StateClosed)
cb.consecutiveFailures.Store(0)
@@ -203,6 +205,7 @@ func (cb *CircuitBreaker) RecordFailure() {
switch state {
case StateClosed:
// Check if we should open the circuit
// #nosec G115 -- MaxFailures is a small config value that fits in int32
if failures >= int32(cb.config.MaxFailures) {
cb.openCircuit()
} else if cb.config.FailureThreshold > 0 {
+2
View File
@@ -217,6 +217,7 @@ func (hc *HealthChecker) recordSuccess(latency time.Duration) {
newStatus := currentStatus
// Check if we should become healthy
// #nosec G115 -- HealthyThreshold is a small config value that fits in int32
if successes >= int32(hc.config.HealthyThreshold) {
if latency > hc.config.DegradedThreshold {
newStatus = HealthDegraded
@@ -241,6 +242,7 @@ func (hc *HealthChecker) recordFailure() {
hc.timeMu.Unlock()
// Check if we should become unhealthy
// #nosec G115 -- UnhealthyThreshold is a small config value that fits in int32
if failures >= int32(hc.config.UnhealthyThreshold) {
hc.setStatus(HealthUnhealthy)
}
+1
View File
@@ -150,6 +150,7 @@ func (h *HealthCheckBackend) IsHealthy() bool {
// recordResult records the result of an operation for health tracking
func (h *HealthCheckBackend) recordResult(success bool) {
// #nosec G115 -- threshold config values are small integers that fit in int32
if success {
fails := h.consecutiveFails.Swap(0)
oks := h.consecutiveOK.Add(1)
+2 -1
View File
@@ -304,7 +304,8 @@ func (f *Factory) createSecureTLSConfig() *tls.Config {
tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
},
InsecureSkipVerify: false, // SECURITY: Always verify certificates
InsecureSkipVerify: false, // SECURITY: Always verify certificates
// #nosec G402 -- PreferServerCipherSuites is deprecated in Go 1.17+ but setting it to false is safe
PreferServerCipherSuites: false, // Let client choose best cipher
}
}
+2
View File
@@ -144,12 +144,14 @@ func getOrCreateLogFile(filename string) io.Writer {
}
// Ensure log directory exists
// #nosec G301 -- log directory needs to be readable by monitoring tools
if err := os.MkdirAll(logDir, 0755); err != nil {
// Fall back to stderr if we can't create the directory
return os.Stderr
}
filepath := logDir + "/" + filename
// #nosec G302 G304 -- log files need to be readable; path is from trusted env var
file, err := os.OpenFile(filepath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
if err != nil {
// Fall back to stderr if we can't open the file
+2
View File
@@ -107,6 +107,7 @@ const (
JWTPattern = `^[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$`
// Bearer token pattern (Authorization header)
// #nosec G101 -- This is a regex pattern for validation, not a hardcoded credential
BearerTokenPattern = `^Bearer\s+([A-Za-z0-9._~+/-]+=*)$`
// Client ID pattern (alphanumeric with common separators)
@@ -119,6 +120,7 @@ const (
SessionIDPattern = `^[a-fA-F0-9]{32,128}$`
// CSRF token pattern (base64url)
// #nosec G101 -- This is a regex pattern for validation, not a hardcoded credential
CSRFTokenPattern = `^[A-Za-z0-9_-]+$`
// Nonce pattern (base64url)
+3 -1
View File
@@ -202,8 +202,10 @@ func (p *TransportPool) createTransport(config TransportConfig) *http.Transport
tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
},
// #nosec G402 -- PreferServerCipherSuites is deprecated in Go 1.17+ but setting it is harmless
PreferServerCipherSuites: true,
InsecureSkipVerify: config.InsecureSkipVerify,
// #nosec G402 -- InsecureSkipVerify is configurable for testing/dev environments
InsecureSkipVerify: config.InsecureSkipVerify,
}
return &http.Transport{
+3
View File
@@ -148,6 +148,7 @@ func (cb *CircuitBreaker) allowRequest() bool {
// allowHalfOpenRequest checks if a request is allowed in half-open state
func (cb *CircuitBreaker) allowHalfOpenRequest() bool {
current := atomic.AddInt32(&cb.halfOpenRequests, 1)
// #nosec G115 -- MaxRequests is a small config value that fits in int32
if current <= int32(cb.config.MaxRequests) {
return true
}
@@ -164,6 +165,7 @@ func (cb *CircuitBreaker) recordFailure() {
state := CircuitBreakerState(atomic.LoadInt32(&cb.state))
// #nosec G115 -- FailureThreshold is a small config value that fits in int32
if state == CircuitBreakerClosed && failures >= int32(cb.config.FailureThreshold) {
cb.transitionToOpen()
} else if state == CircuitBreakerHalfOpen {
@@ -180,6 +182,7 @@ func (cb *CircuitBreaker) recordSuccess() {
state := CircuitBreakerState(atomic.LoadInt32(&cb.state))
// #nosec G115 -- SuccessThreshold is a small config value that fits in int32
if state == CircuitBreakerHalfOpen && successes >= int32(cb.config.SuccessThreshold) {
cb.transitionToClosed()
}
+1
View File
@@ -191,6 +191,7 @@ func (re *RetryExecutor) calculateDelay(attempt int) time.Duration {
}
// Add jitter
// #nosec G404 -- math/rand is acceptable for jitter timing, not security-sensitive
if re.config.RandomizationFactor > 0 {
jitter := delay * re.config.RandomizationFactor
minDelay := delay - jitter