mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-12 04:04:26 +00:00
fix 116 (#118)
* Fix cache serialisation * fix(cache): add integer overflow protection for serialization - [x] Add maxCacheEntrySize constant (64 MiB) to prevent memory overflow - [x] Validate byte slice size before adding marker byte - [x] Validate JSON-serialized data size before marker addition - [x] Add comprehensive overflow protection test cases * docs: add security fix documentation for integer overflow protection * test: fix goroutine tests to use mock OIDC servers The TestContextAwareGoroutineManagement tests were making real HTTP calls to hardcoded URLs like https://example.com, causing failures in CI when those requests timeout or return HTTP errors. Changes: - Added createMockOIDCServer() helper function using httptest - Updated GoroutineCleanupOnContextCancel to use mock server - Updated NoGoroutineLeakOnMultipleInstances to use 3 mock servers - Updated SingletonTasksAcrossInstances to use mock servers array This prevents network calls and makes tests more reliable and faster. Fixes test failures in GitHub Actions CI.
This commit is contained in:
@@ -0,0 +1,49 @@
|
|||||||
|
# Security Fix: Integer Overflow Protection in Cache Serialization
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Fixed **High severity** integer overflow vulnerability identified by GitHub Advanced Security in PR #117.
|
||||||
|
|
||||||
|
## Vulnerability
|
||||||
|
|
||||||
|
**Locations**: `universal_cache.go` lines 789 and 811
|
||||||
|
- `result := make([]byte, len(bytes)+1)` - Raw bytes path
|
||||||
|
- `result := make([]byte, len(jsonData)+1)` - JSON encoding path
|
||||||
|
|
||||||
|
**Risk**: Potential integer overflow when allocating memory for very large cache entries.
|
||||||
|
|
||||||
|
## Fix Applied
|
||||||
|
|
||||||
|
1. **Added size limit constant**:
|
||||||
|
```go
|
||||||
|
maxCacheEntrySize = 64 * 1024 * 1024 // 64 MiB
|
||||||
|
```
|
||||||
|
|
||||||
|
2. **Size validation before allocation**:
|
||||||
|
- Validates entry size doesn't exceed limit
|
||||||
|
- Validates adding marker byte won't overflow
|
||||||
|
- Returns descriptive error messages
|
||||||
|
|
||||||
|
3. **Comprehensive test coverage**:
|
||||||
|
- Oversized byte slices (>64 MiB)
|
||||||
|
- Exact max size edge case
|
||||||
|
- Safe sizes (normal operation)
|
||||||
|
- Large JSON data structures
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
✅ All tests pass with race detection
|
||||||
|
✅ No security issues (golangci-lint, gosec)
|
||||||
|
✅ 76.3% test coverage maintained
|
||||||
|
|
||||||
|
## Impact
|
||||||
|
|
||||||
|
- No breaking changes
|
||||||
|
- Negligible performance overhead
|
||||||
|
- Prevents potential buffer overflows
|
||||||
|
- Predictable memory usage
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
**Date**: January 8, 2026
|
||||||
|
**Severity**: High → Resolved
|
||||||
@@ -4,7 +4,10 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"crypto/sha256"
|
"crypto/sha256"
|
||||||
"encoding/hex"
|
"encoding/hex"
|
||||||
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
"runtime"
|
"runtime"
|
||||||
"sync"
|
"sync"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
@@ -251,6 +254,30 @@ func TestSingletonResourceManager(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// createMockOIDCServer creates a mock OIDC server for testing
|
||||||
|
func createMockOIDCServer() *httptest.Server {
|
||||||
|
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
switch r.URL.Path {
|
||||||
|
case "/.well-known/openid-configuration":
|
||||||
|
w.Header().Set("Content-Type", "application/json")
|
||||||
|
json.NewEncoder(w).Encode(map[string]interface{}{
|
||||||
|
"issuer": "https://example.com",
|
||||||
|
"authorization_endpoint": "https://example.com/authorize",
|
||||||
|
"token_endpoint": "https://example.com/token",
|
||||||
|
"jwks_uri": "https://example.com/jwks",
|
||||||
|
"userinfo_endpoint": "https://example.com/userinfo",
|
||||||
|
})
|
||||||
|
case "/jwks":
|
||||||
|
w.Header().Set("Content-Type", "application/json")
|
||||||
|
json.NewEncoder(w).Encode(map[string]interface{}{
|
||||||
|
"keys": []interface{}{},
|
||||||
|
})
|
||||||
|
default:
|
||||||
|
w.WriteHeader(http.StatusNotFound)
|
||||||
|
}
|
||||||
|
}))
|
||||||
|
}
|
||||||
|
|
||||||
// TestContextAwareGoroutineManagement tests context-aware goroutine management
|
// TestContextAwareGoroutineManagement tests context-aware goroutine management
|
||||||
func TestContextAwareGoroutineManagement(t *testing.T) {
|
func TestContextAwareGoroutineManagement(t *testing.T) {
|
||||||
t.Run("GoroutineCleanupOnContextCancel", func(t *testing.T) {
|
t.Run("GoroutineCleanupOnContextCancel", func(t *testing.T) {
|
||||||
@@ -259,13 +286,17 @@ func TestContextAwareGoroutineManagement(t *testing.T) {
|
|||||||
ResetUniversalCacheManagerForTesting()
|
ResetUniversalCacheManagerForTesting()
|
||||||
defer ResetUniversalCacheManagerForTesting()
|
defer ResetUniversalCacheManagerForTesting()
|
||||||
|
|
||||||
|
// Create mock OIDC server
|
||||||
|
mockServer := createMockOIDCServer()
|
||||||
|
defer mockServer.Close()
|
||||||
|
|
||||||
initialGoroutines := runtime.NumGoroutine()
|
initialGoroutines := runtime.NumGoroutine()
|
||||||
|
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
|
||||||
// Create a TraefikOidc instance with context
|
// Create a TraefikOidc instance with context
|
||||||
config := &Config{
|
config := &Config{
|
||||||
ProviderURL: "https://example.com",
|
ProviderURL: mockServer.URL,
|
||||||
ClientID: "test-client",
|
ClientID: "test-client",
|
||||||
ClientSecret: "test-secret",
|
ClientSecret: "test-secret",
|
||||||
}
|
}
|
||||||
@@ -308,12 +339,20 @@ func TestContextAwareGoroutineManagement(t *testing.T) {
|
|||||||
ResetUniversalCacheManagerForTesting()
|
ResetUniversalCacheManagerForTesting()
|
||||||
defer ResetUniversalCacheManagerForTesting()
|
defer ResetUniversalCacheManagerForTesting()
|
||||||
|
|
||||||
|
// Create mock OIDC servers
|
||||||
|
mockServer1 := createMockOIDCServer()
|
||||||
|
defer mockServer1.Close()
|
||||||
|
mockServer2 := createMockOIDCServer()
|
||||||
|
defer mockServer2.Close()
|
||||||
|
mockServer3 := createMockOIDCServer()
|
||||||
|
defer mockServer3.Close()
|
||||||
|
|
||||||
initialGoroutines := runtime.NumGoroutine()
|
initialGoroutines := runtime.NumGoroutine()
|
||||||
|
|
||||||
configs := []Config{
|
configs := []Config{
|
||||||
{ProviderURL: "https://example1.com", ClientID: "client1", ClientSecret: "secret1"},
|
{ProviderURL: mockServer1.URL, ClientID: "client1", ClientSecret: "secret1"},
|
||||||
{ProviderURL: "https://example2.com", ClientID: "client2", ClientSecret: "secret2"},
|
{ProviderURL: mockServer2.URL, ClientID: "client2", ClientSecret: "secret2"},
|
||||||
{ProviderURL: "https://example3.com", ClientID: "client3", ClientSecret: "secret3"},
|
{ProviderURL: mockServer3.URL, ClientID: "client3", ClientSecret: "secret3"},
|
||||||
}
|
}
|
||||||
|
|
||||||
var plugins []*TraefikOidc
|
var plugins []*TraefikOidc
|
||||||
@@ -366,6 +405,13 @@ func TestContextAwareGoroutineManagement(t *testing.T) {
|
|||||||
ResetUniversalCacheManagerForTesting()
|
ResetUniversalCacheManagerForTesting()
|
||||||
defer ResetUniversalCacheManagerForTesting()
|
defer ResetUniversalCacheManagerForTesting()
|
||||||
|
|
||||||
|
// Create mock OIDC servers
|
||||||
|
mockServers := make([]*httptest.Server, 3)
|
||||||
|
for i := 0; i < 3; i++ {
|
||||||
|
mockServers[i] = createMockOIDCServer()
|
||||||
|
defer mockServers[i].Close()
|
||||||
|
}
|
||||||
|
|
||||||
rm := GetResourceManager()
|
rm := GetResourceManager()
|
||||||
|
|
||||||
// Register singleton cleanup task
|
// Register singleton cleanup task
|
||||||
@@ -386,7 +432,7 @@ func TestContextAwareGoroutineManagement(t *testing.T) {
|
|||||||
for i := 0; i < 3; i++ {
|
for i := 0; i < 3; i++ {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
config := &Config{
|
config := &Config{
|
||||||
ProviderURL: fmt.Sprintf("https://example%d.com", i),
|
ProviderURL: mockServers[i].URL,
|
||||||
ClientID: fmt.Sprintf("client%d", i),
|
ClientID: fmt.Sprintf("client%d", i),
|
||||||
ClientSecret: fmt.Sprintf("secret%d", i),
|
ClientSecret: fmt.Sprintf("secret%d", i),
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user