random: seed math/rand in one place with crypto strong seed #4783

This shouldn't be read as encouraging the use of math/rand instead of
crypto/rand in security sensitive contexts, rather as a safer default
if that does happen by accident.

backend/union/policy/eprand.go

@@ -3,7 +3,6 @@ package policy
 import (
 	"context"
 	"math/rand"
-	"time"
 
 	"github.com/rclone/rclone/backend/union/upstream"
 	"github.com/rclone/rclone/fs"
@@ -20,12 +19,10 @@ type EpRand struct {
 }
 
 func (p *EpRand) rand(upstreams []*upstream.Fs) *upstream.Fs {
-	rand.Seed(time.Now().Unix())
 	return upstreams[rand.Intn(len(upstreams))]
 }
 
 func (p *EpRand) randEntries(entries []upstream.Entry) upstream.Entry {
-	rand.Seed(time.Now().Unix())
 	return entries[rand.Intn(len(entries))]
 }
 

cmd/cmd.go

@@ -10,7 +10,6 @@ import (
 	"context"
 	"fmt"
 	"log"
-	"math/rand"
 	"os"
 	"os/exec"
 	"path"
@@ -37,6 +36,7 @@ import (
 	"github.com/rclone/rclone/fs/rc/rcflags"
 	"github.com/rclone/rclone/fs/rc/rcserver"
 	"github.com/rclone/rclone/lib/atexit"
+	"github.com/rclone/rclone/lib/random"
 	"github.com/rclone/rclone/lib/terminal"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -533,7 +533,9 @@ func AddBackendFlags() {
 
 // Main runs rclone interpreting flags and commands out of os.Args
 func Main() {
-	rand.Seed(time.Now().Unix())
+	if err := random.Seed(); err != nil {
+		log.Fatalf("Fatal error: %v", err)
+	}
 	setupRootCommand(Root)
 	AddBackendFlags()
 	if err := Root.Execute(); err != nil {

lib/random/random.go

@@ -4,6 +4,7 @@ package random
 import (
 	cryptorand "crypto/rand"
 	"encoding/base64"
+	"encoding/binary"
 	mathrand "math/rand"
 
 	"github.com/pkg/errors"
@@ -52,3 +53,19 @@ func Password(bits int) (password string, err error) {
 	password = base64.RawURLEncoding.EncodeToString(pw)
 	return password, nil
 }
+
+// Seed the global math/rand with crypto strong data
+//
+// This doesn't make it OK to use math/rand in crypto sensitive
+// environments - don't do that! However it does help to mitigate the
+// problem if that happens accidentally. This would have helped with
+// CVE-2020-28924 - #4783
+func Seed() error {
+	var seed int64
+	err := binary.Read(cryptorand.Reader, binary.LittleEndian, &seed)
+	if err != nil {
+		return errors.Wrap(err, "failed to read random seed")
+	}
+	mathrand.Seed(seed)
+	return nil
+}

lib/random/random_test.go

@@ -1,6 +1,7 @@
 package random
 
 import (
+	"math/rand"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -48,3 +49,16 @@ func TestPasswordDuplicates(t *testing.T) {
 		seen[s] = true
 	}
 }
+
+func TestSeed(t *testing.T) {
+	// seed 100 times and check the first random number doesn't repeat
+	// This test could fail with a probability of ~ 10**-15
+	const n = 100
+	var seen = map[int64]bool{}
+	for i := 0; i < n; i++ {
+		assert.NoError(t, Seed())
+		first := rand.Int63()
+		assert.False(t, seen[first])
+		seen[first] = true
+	}
+}