Skip to content

Commit d482dc5

Browse files
committed
fixing cwe-338 on other places to address race test failure
1 parent 0b584b7 commit d482dc5

File tree

3 files changed

+38
-17
lines changed

3 files changed

+38
-17
lines changed

child/child.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@
44
package child
55

66
import (
7+
cryptorand "crypto/rand"
78
"errors"
89
"fmt"
910
"io"
1011
"log"
11-
"math/rand"
12+
"math/big"
1213
"os"
1314
"os/exec"
1415
"strings"
@@ -475,8 +476,15 @@ func (c *Child) randomSplay() <-chan time.Time {
475476
}
476477

477478
ns := c.splay.Nanoseconds()
478-
offset := rand.Int63n(ns)
479-
t := time.Duration(offset)
479+
// Use crypto/rand for secure random generation (CWE-338 fix)
480+
max := big.NewInt(ns)
481+
n, err := cryptorand.Int(cryptorand.Reader, max)
482+
if err != nil {
483+
// Fallback to no splay if crypto/rand fails
484+
log.Printf("[WARN] (child) failed to generate secure random splay: %v", err)
485+
return time.After(0)
486+
}
487+
t := time.Duration(n.Int64())
480488

481489
c.logger.Printf("[DEBUG] (child) waiting %.2fs for random splay", t.Seconds())
482490

dependency/vault_common.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"fmt"
1010
"log"
1111
"math/big"
12-
mathrand "math/rand"
1312
"sync"
1413
"time"
1514

@@ -187,15 +186,15 @@ func leaseCheckWait(s *Secret, retryCount int) time.Duration {
187186
sleep = sleep / 3.0
188187

189188
// Use some randomness so many clients do not hit Vault simultaneously.
190-
sleep = sleep * (mathrand.Float64() + 1) / 2.0
189+
sleep = sleep * (secureRandomFloat64() + 1) / 2.0
191190
} else if !rotatingSecret {
192191
// If the secret doesn't have a rotation period, this is a non-renewable leased
193192
// secret.
194193
// For non-renewable leases set the renew duration to use much of the secret
195194
// lease as possible. Use a stagger over the configured threshold
196195
// fraction of the lease duration so that many clients do not hit
197196
// Vault simultaneously.
198-
finalFraction := VaultLeaseRenewalThreshold + (mathrand.Float64()-0.5)*0.1
197+
finalFraction := VaultLeaseRenewalThreshold + (secureRandomFloat64()-0.5)*0.1
199198
if finalFraction >= 1.0 || finalFraction <= 0.0 {
200199
// If the fraction randomly winds up outside of (0.0-1.0), clamp
201200
// back down to the VaultLeaseRenewalThreshold provided by the user,
@@ -210,19 +209,24 @@ func leaseCheckWait(s *Secret, retryCount int) time.Duration {
210209
return time.Duration(sleep)
211210
}
212211

213-
// jitter adds randomness to a duration to prevent thundering herd.
214-
// It reduces the duration by up to maxJitter (10%) randomly using crypto/rand.
215-
// using this to fix CWE-338: Use of Cryptographically Secure Pseudo-Random Number Generator (CSPRNG)
216-
func jitter(t time.Duration) time.Duration {
217-
// Generate cryptographically secure random value between 0.0 and 1.0
212+
// secureRandomFloat64 generates a cryptographically secure random float64 in [0.0, 1.0).
213+
// This is thread-safe and used to fix CWE-338.
214+
func secureRandomFloat64() float64 {
218215
max := big.NewInt(1000000)
219216
n, err := cryptorand.Int(cryptorand.Reader, max)
220217
if err != nil {
221-
// Fallback to no jitter if crypto/rand fails
222-
log.Printf("[WARN] Failed to generate secure random jitter: %v", err)
223-
return t
218+
// Fallback to 0.5 if crypto/rand fails (rare but possible)
219+
log.Printf("[WARN] Failed to generate secure random number: %v", err)
220+
return 0.5
224221
}
225-
randomFloat := float64(n.Int64()) / 1000000.0
222+
return float64(n.Int64()) / 1000000.0
223+
}
224+
225+
// jitter adds randomness to a duration to prevent thundering herd.
226+
// It reduces the duration by up to maxJitter (10%) randomly using crypto/rand.
227+
// This function is thread-safe.
228+
func jitter(t time.Duration) time.Duration {
229+
randomFloat := secureRandomFloat64()
226230
f := float64(t) * (1.0 - maxJitter*randomFloat)
227231
return time.Duration(f)
228232
}

watch/view.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@
44
package watch
55

66
import (
7+
cryptorand "crypto/rand"
78
"errors"
89
"fmt"
910
"log"
10-
"math/rand"
11+
"math/big"
1112
"reflect"
1213
"sync"
1314
"time"
@@ -311,7 +312,15 @@ const minDelayBetweenUpdates = time.Millisecond * 100
311312
func rateLimiter(start time.Time) time.Duration {
312313
remaining := minDelayBetweenUpdates - time.Since(start)
313314
if remaining > 0 {
314-
dither := time.Duration(rand.Int63n(20000000)) // 0-20ms
315+
// Use crypto/rand for secure random generation (CWE-338 fix)
316+
max := big.NewInt(20000000) // 0-20ms in nanoseconds
317+
n, err := cryptorand.Int(cryptorand.Reader, max)
318+
if err != nil {
319+
// Fallback to no dither if crypto/rand fails
320+
log.Printf("[WARN] (view) failed to generate secure random dither: %v", err)
321+
return remaining
322+
}
323+
dither := time.Duration(n.Int64())
315324
return remaining + dither
316325
}
317326
return 0

0 commit comments

Comments
 (0)