From e3eff7c0625f82bf2e6dde1539699296a5415ae0 Mon Sep 17 00:00:00 2001
From: jbauch <jbauch@webrtc.org>
Date: Mon, 8 Aug 2016 17:13:33 -0700
Subject: [PATCH] Check that table evenly divides 256 when generating random
 string.

This prevents biased modulo arithmetic when selecting a character for a
random value from the provided table.

BUG=webrtc:5870

Review-Url: https://codereview.webrtc.org/2115793003
Cr-Commit-Position: refs/heads/master@{#13683}
---
 webrtc/base/helpers.cc          |  7 ++++++-
 webrtc/base/helpers.h           |  2 ++
 webrtc/base/helpers_unittest.cc | 15 +++++++++++++--
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/webrtc/base/helpers.cc b/webrtc/base/helpers.cc
index 9674eb9859..a8389d462e 100644
--- a/webrtc/base/helpers.cc
+++ b/webrtc/base/helpers.cc
@@ -207,10 +207,15 @@ std::string CreateRandomString(size_t len) {
   return str;
 }
 
-bool CreateRandomString(size_t len,
+static bool CreateRandomString(size_t len,
                         const char* table, int table_size,
                         std::string* str) {
   str->clear();
+  // Avoid biased modulo division below.
+  if (256 % table_size) {
+    LOG(LS_ERROR) << "Table size must divide 256 evenly!";
+    return false;
+  }
   std::unique_ptr<uint8_t[]> bytes(new uint8_t[len]);
   if (!Rng().Generate(bytes.get(), len)) {
     LOG(LS_ERROR) << "Failed to generate random string!";
diff --git a/webrtc/base/helpers.h b/webrtc/base/helpers.h
index 2e72dfb26e..fcf77afd7f 100644
--- a/webrtc/base/helpers.h
+++ b/webrtc/base/helpers.h
@@ -35,6 +35,8 @@ bool CreateRandomString(size_t length, std::string* str);
 // Generates a (cryptographically) random string of the given length,
 // with characters from the given table. Return false if the random
 // number generator failed.
+// For ease of implementation, the function requires that the table
+// size evenly divide 256; otherwise, it returns false.
 bool CreateRandomString(size_t length, const std::string& table,
                         std::string* str);
 
diff --git a/webrtc/base/helpers_unittest.cc b/webrtc/base/helpers_unittest.cc
index e4903f5869..394e2b0dd4 100644
--- a/webrtc/base/helpers_unittest.cc
+++ b/webrtc/base/helpers_unittest.cc
@@ -55,6 +55,17 @@ TEST_F(RandomTest, TestCreateRandomData) {
   EXPECT_NE(0, memcmp(random1.data(), random2.data(), kRandomDataLength));
 }
 
+TEST_F(RandomTest, TestCreateRandomStringEvenlyDivideTable) {
+  static std::string kUnbiasedTable("01234567");
+  std::string random;
+  EXPECT_TRUE(CreateRandomString(256, kUnbiasedTable, &random));
+  EXPECT_EQ(256U, random.size());
+
+  static std::string kBiasedTable("0123456789");
+  EXPECT_FALSE(CreateRandomString(256, kBiasedTable, &random));
+  EXPECT_EQ(0U, random.size());
+}
+
 TEST_F(RandomTest, TestCreateRandomUuid) {
   std::string random = CreateRandomUuid();
   EXPECT_EQ(36U, random.size());
@@ -90,8 +101,8 @@ TEST_F(RandomTest, TestCreateRandomForTest) {
   std::string str;
   EXPECT_TRUE(CreateRandomString(16, "a", &str));
   EXPECT_EQ("aaaaaaaaaaaaaaaa", str);
-  EXPECT_TRUE(CreateRandomString(16, "abc", &str));
-  EXPECT_EQ("acbccaaaabbaacbb", str);
+  EXPECT_TRUE(CreateRandomString(16, "abcd", &str));
+  EXPECT_EQ("dbaaabdaccbcabbd", str);
 
   // Turn off test mode for other tests.
   SetRandomTestMode(false);