CVE-2018-6337: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: folly::secureRandom will re-use a buffer between parent and child processes when fork() is called. That will result in multiple forked children producing repeat (or similar) results. This affects HHVM 3.26 prior to 3.26.3 and the folly library between v2017.12.11.00 and v2018.08.09.00.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 7.5
Published: 2018-12-31T22:29Z
Repository: https://github.com/facebook/folly
Language: C++
Files changed: folly/Random.cpp, folly/test/RandomTest.cpp
Commit date: 2018-08-08 17:18:14 -0700
Commit message: Flush secureRandom buffer on fork

Summary: On fork, flush the secureRandom buffer, so that we don't share entropy between the parent and child.

Reviewed By: ricklavoie

Differential Revision: D9196474

fbshipit-source-id: 12ff8488d814466186df61328a5f1d4000beb27f

Diff: diff --git a/folly/Random.cpp b/folly/Random.cpp
index afb62f5d..40a6b17f 100644
--- a/folly/Random.cpp
+++ b/folly/Random.cpp
@@ -70,10 +70,11 @@ void readRandomDevice(void* data, size_t size) {
 #endif
 }
 
 class BufferedRandomDevice {
  public:
+  static once_flag flag;
   static constexpr size_t kDefaultBufferSize = 128;
 
   explicit BufferedRandomDevice(size_t bufferSize = kDefaultBufferSize);
 
   void get(void* data, size_t size) {
@@ -95,14 +96,32 @@ class BufferedRandomDevice {
   const size_t bufferSize_;
   std::unique_ptr<unsigned char[]> buffer_;
   unsigned char* ptr_;
 };
 
+once_flag BufferedRandomDevice::flag;
+struct RandomTag {};
+
 BufferedRandomDevice::BufferedRandomDevice(size_t bufferSize)
   : bufferSize_(bufferSize),
     buffer_(new unsigned char[bufferSize]),
     ptr_(buffer_.get() + bufferSize) {  // refill on first use
+  call_once(flag, [this]() {
+    detail::AtFork::registerHandler(
+        this,
+        /*prepare*/ []() { return true; },
+        /*parent*/ []() {},
+        /*child*/
+        []() {
+          using Single = SingletonThreadLocal<BufferedRandomDevice, RandomTag>;
+          auto& t = Single::get();
+          // Clear out buffered data on fork.
+          //
+          // Ensure child and parent do not share same entropy pool.
+          t.ptr_ = t.buffer_.get() + t.bufferSize_;
+        });
+  });
 }
 
 void BufferedRandomDevice::getSlow(unsigned char* data, size_t size) {
   DCHECK_GT(size, remaining());
   if (size >= bufferSize_) {
@@ -122,12 +141,10 @@ void BufferedRandomDevice::getSlow(unsigned char* data, size_t size) {
 
   memcpy(data, ptr_, size);
   ptr_ += size;
 }
 
-struct RandomTag {};
-
 } // namespace
 
 void Random::secureRandom(void* data, size_t size) {
   using Single = SingletonThreadLocal<BufferedRandomDevice, RandomTag>;
   Single::get().get(data, size);
diff --git a/folly/test/RandomTest.cpp b/folly/test/RandomTest.cpp
index 9c33159b..2ee74c6d 100644
--- a/folly/test/RandomTest.cpp
+++ b/folly/test/RandomTest.cpp
@@ -136,5 +136,26 @@ TEST(Random, sanity) {
     EXPECT_EQ(
         vals.size(),
         std::unordered_set<uint64_t>(vals.begin(), vals.end()).size());
   }
 }
+
+TEST(Random, SecureFork) {
+  unsigned char buffer = 0;
+  // Init random buffer
+  folly::Random::secureRandom(&buffer, 1);
+
+  auto pid = fork();
+  EXPECT_NE(pid, -1);
+  if (pid) {
+    // parent
+    int status = 0;
+    folly::Random::secureRandom(&buffer, 1);
+    auto pid2 = wait(&status);
+    EXPECT_NE(WEXITSTATUS(status), buffer);
+    EXPECT_EQ(pid, pid2);
+  } else {
+    // child
+    folly::Random::secureRandom(&buffer, 1);
+    exit(buffer); // Do not print gtest results
+  }
+}

Vulnerable code: struct RandomTag {};

Fixed code:   static once_flag flag;
once_flag BufferedRandomDevice::flag;
struct RandomTag {};

  call_once(flag, [this]() {
    detail::AtFork::registerHandler(
        this,
        /*prepare*/ []() { return true; },
        /*parent*/ []() {},
        /*child*/
        []() {
          using Single = SingletonThreadLocal<BufferedRandomDevice, RandomTag>;
          auto& t = Single::get();
          // Clear out buffered data on fork.
          //
          // Ensure child and parent do not share same entropy pool.
          t.ptr_ = t.buffer_.get() + t.bufferSize_;
        });
  });

TEST(Random, SecureFork) {
  unsigned char buffer = 0;
  // Init random buffer
  folly::Random::secureRandom(&buffer, 1);

  auto pid = fork();
  EXPECT_NE(pid, -1);
  if (pid) {
    // parent
    int status = 0;
    folly::Random::secureRandom(&buffer, 1);
    auto pid2 = wait(&status);
    EXPECT_NE(WEXITSTATUS(status), buffer);
    EXPECT_EQ(pid, pid2);
  } else {
    // child
    folly::Random::secureRandom(&buffer, 1);
    exit(buffer); // Do not print gtest results
  }
}