From 69f3c942bea36d07e832af38e3aa36c326491c30 Mon Sep 17 00:00:00 2001
From: Alan Frindell <afrind@fb.com>
Date: Thu, 22 Oct 2015 13:12:58 -0700
Subject: [PATCH] Fix infinite loop in Cursor::readTerminatedString

Summary: readTerminatedString could infinite loop if the terminator does not appear in the contained IOBuf chain and maxLength > chain.computeChainLength.  I'm throwing out_of_range here because that more closely mirrors what the other read() functions do.

Reviewed By: siyengar

Differential Revision: D2571039

fb-gh-sync-id: 1db22089562d8767920d66a0a1b091b02de6571f
---
 folly/io/Cursor.h                 |  3 ++-
 folly/io/test/IOBufCursorTest.cpp | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/folly/io/Cursor.h b/folly/io/Cursor.h
index 5d12b6f8..d38d4dfb 100644
--- a/folly/io/Cursor.h
+++ b/folly/io/Cursor.h
@@ -212,7 +212,7 @@ class CursorBase {
       size_t maxLength = std::numeric_limits<size_t>::max()) {
     std::string str;
 
-    for (;;) {
+    while (!isAtEnd()) {
       const uint8_t* buf = data();
       size_t buflen = length();
 
@@ -235,6 +235,7 @@ class CursorBase {
 
       skip(i);
     }
+    throw std::out_of_range("terminator not found");
   }
 
   size_t skipAtMost(size_t len) {
diff --git a/folly/io/test/IOBufCursorTest.cpp b/folly/io/test/IOBufCursorTest.cpp
index 86aad240..31801b8a 100644
--- a/folly/io/test/IOBufCursorTest.cpp
+++ b/folly/io/test/IOBufCursorTest.cpp
@@ -683,6 +683,33 @@ TEST(IOBuf, StringOperations) {
     EXPECT_STREQ("hello", curs.readTerminatedString().c_str());
   }
 
+  // Test reading a null-terminated string from a chain that doesn't contain the
+  // terminator
+  {
+    std::unique_ptr<IOBuf> buf(IOBuf::create(8));
+    Appender app(buf.get(), 0);
+    app.push(reinterpret_cast<const uint8_t*>("hello"), 5);
+    std::unique_ptr<IOBuf> chain(IOBuf::create(8));
+    chain->prependChain(std::move(buf));
+
+    Cursor curs(chain.get());
+    EXPECT_THROW(curs.readTerminatedString(),
+                 std::out_of_range);
+  }
+
+  // Test reading a null-terminated string past the maximum length
+  {
+    std::unique_ptr<IOBuf> buf(IOBuf::create(8));
+    Appender app(buf.get(), 0);
+    app.push(reinterpret_cast<const uint8_t*>("hello\0"), 6);
+    std::unique_ptr<IOBuf> chain(IOBuf::create(8));
+    chain->prependChain(std::move(buf));
+
+    Cursor curs(chain.get());
+    EXPECT_THROW(curs.readTerminatedString('\0', 3),
+                 std::length_error);
+  }
+
   // Test reading a two fixed-length strings from a single buffer with an extra
   // uint8_t at the end
   {
-- 
2.34.1