fixing race condition and cwe-338

Author: aswanidutt

child/child.go

@@ -4,11 +4,12 @@
 package child
 
 import (
+	"crypto/rand"
 	"errors"
 	"fmt"
 	"io"
 	"log"
-	"math/rand"
+	"math/big"
 	"os"
 	"os/exec"
 	"strings"
@@ -475,8 +476,14 @@ func (c *Child) randomSplay() <-chan time.Time {
 	}
 
 	ns := c.splay.Nanoseconds()
-	offset := rand.Int63n(ns)
-	t := time.Duration(offset)
+	// Use crypto/rand for secure random generation (CWE-338 fix)
+	n, err := rand.Int(rand.Reader, big.NewInt(ns))
+	if err != nil {
+		// Fallback to no splay on error
+		c.logger.Printf("[WARN] (child) failed to generate random splay: %v", err)
+		return time.After(0)
+	}
+	t := time.Duration(n.Int64())
 
 	c.logger.Printf("[DEBUG] (child) waiting %.2fs for random splay", t.Seconds())
 

dependency/file_test.go

@@ -6,6 +6,7 @@ package dependency
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -136,15 +137,39 @@ func TestFileQuery_Fetch(t *testing.T) {
 		}
 	})
 
-	syncWriteFile := func(name string, data []byte, perm os.FileMode) error {
-		f, err := os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_TRUNC|os.O_SYNC, perm)
+	syncWriteFile := func(name string, data []byte, _ os.FileMode) error {
+		// Write to a temporary file in the same directory to ensure atomic rename
+		dir := filepath.Dir(name)
+		tmpFile, err := os.CreateTemp(dir, ".syncwrite-*")
+		if err != nil {
+			return err
+		}
+		tmpName := tmpFile.Name()
+		defer os.Remove(tmpName)
+
+		_, err = tmpFile.Write(data)
 		if err == nil {
-			_, err = f.Write(data)
-			if err1 := f.Close(); err1 != nil && err == nil {
-				err = err1
-			}
+			err = tmpFile.Sync()
+		}
+		if err1 := tmpFile.Close(); err1 != nil && err == nil {
+			err = err1
+		}
+		if err != nil {
+			return err
+		}
+
+		// Atomically rename the temp file to the target file
+		if err := os.Rename(tmpName, name); err != nil {
+			return err
 		}
-		return err
+
+		// Ensure the directory entry is synced (best effort)
+		if dirFile, err := os.Open(dir); err == nil {
+			dirFile.Sync()
+			dirFile.Close()
+		}
+
+		return nil
 	}
 	t.Run("fires_changes", func(t *testing.T) {
 		f, err := os.CreateTemp("", "")

dependency/vault_common.go

@@ -4,10 +4,11 @@
 package dependency
 
 import (
+	"crypto/rand"
 	"encoding/json"
 	"fmt"
 	"log"
-	"math/rand"
+	"math/big"
 	"sync"
 	"time"
 
@@ -77,6 +78,21 @@ type renewer interface {
 	secrets() (*Secret, *api.Secret)
 }
 
+// cryptoRandFloat64 generates a cryptographically secure random float64 in [0.0, 1.0)
+func cryptoRandFloat64() float64 {
+	// Generate a random 53-bit integer (mantissa precision of float64)
+	max := big.NewInt(1 << 53)
+	n, err := rand.Int(rand.Reader, max)
+	if err != nil {
+		// Fallback to a reasonable default if crypto/rand fails
+		// This should never happen in practice
+		log.Printf("[WARN] crypto/rand failed, using 0.5 as fallback: %v", err)
+		return 0.5
+	}
+	// Convert to float64 in range [0.0, 1.0)
+	return float64(n.Int64()) / float64(max.Int64())
+}
+
 func renewSecret(clients *ClientSet, d renewer) error {
 	log.Printf("[TRACE] %s: starting renewer", d)
 
@@ -166,15 +182,15 @@ func leaseCheckWait(s *Secret) time.Duration {
 		sleep = sleep / 3.0
 
 		// Use some randomness so many clients do not hit Vault simultaneously.
-		sleep = sleep * (rand.Float64() + 1) / 2.0
+		sleep = sleep * (cryptoRandFloat64() + 1) / 2.0
 	} else if !rotatingSecret {
 		// If the secret doesn't have a rotation period, this is a non-renewable leased
 		// secret.
 		// For non-renewable leases set the renew duration to use much of the secret
 		// lease as possible. Use a stagger over the configured threshold
 		// fraction of the lease duration so that many clients do not hit
 		// Vault simultaneously.
-		finalFraction := VaultLeaseRenewalThreshold + (rand.Float64()-0.5)*0.1
+		finalFraction := VaultLeaseRenewalThreshold + (cryptoRandFloat64()-0.5)*0.1
 		if finalFraction >= 1.0 || finalFraction <= 0.0 {
 			// If the fraction randomly winds up outside of (0.0-1.0), clamp
 			// back down to the VaultLeaseRenewalThreshold provided by the user,

watch/view.go

@@ -4,10 +4,11 @@
 package watch
 
 import (
+	"crypto/rand"
 	"errors"
 	"fmt"
 	"log"
-	"math/rand"
+	"math/big"
 	"reflect"
 	"sync"
 	"time"
@@ -311,7 +312,13 @@ const minDelayBetweenUpdates = time.Millisecond * 100
 func rateLimiter(start time.Time) time.Duration {
 	remaining := minDelayBetweenUpdates - time.Since(start)
 	if remaining > 0 {
-		dither := time.Duration(rand.Int63n(20000000)) // 0-20ms
+		// Use crypto/rand for secure random generation (CWE-338 fix)
+		n, err := rand.Int(rand.Reader, big.NewInt(20000000))
+		if err != nil {
+			// Fallback to no dither on error
+			return remaining
+		}
+		dither := time.Duration(n.Int64()) // 0-20ms in nanoseconds
 		return remaining + dither
 	}
 	return 0