[PATCH] D72409: [clang] Fix out-of-bounds memory access in ComputeLineNumbers

[Jan Korous via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf28972facc1f: [clang] Fix out-of-bounds memory access in ComputeLineNumbers (authored by jkorous).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D72409?vs=237219&id=237399#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72409/new/

https://reviews.llvm.org/D72409

Files:
  clang/lib/Basic/SourceManager.cpp
  clang/unittests/Basic/SourceManagerTest.cpp


Index: clang/unittests/Basic/SourceManagerTest.cpp
===================================================================
--- clang/unittests/Basic/SourceManagerTest.cpp
+++ clang/unittests/Basic/SourceManagerTest.cpp
@@ -20,7 +20,9 @@
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/Process.h"
 #include "gtest/gtest.h"
+#include <cstddef>
 
 using namespace clang;
 
@@ -241,6 +243,28 @@
             "UTF-32 (LE)");
 }
 
+// Regression test - there was an out of bound access for buffers not terminated by zero.
+TEST_F(SourceManagerTest, getLineNumber) {
+  const unsigned pageSize = llvm::sys::Process::getPageSizeEstimate();
+  std::unique_ptr<char[]> source(new char[pageSize]);
+  for(unsigned i = 0; i < pageSize; ++i) {
+    source[i] = 'a';
+  }
+
+  std::unique_ptr<llvm::MemoryBuffer> Buf =
+      llvm::MemoryBuffer::getMemBuffer(
+        llvm::MemoryBufferRef(
+          llvm::StringRef(source.get(), 3), "whatever"
+        ),
+        false
+      );
+
+  FileID mainFileID = SourceMgr.createFileID(std::move(Buf));
+  SourceMgr.setMainFileID(mainFileID);
+
+  ASSERT_NO_FATAL_FAILURE(SourceMgr.getLineNumber(mainFileID, 1, nullptr));
+}
+
 #if defined(LLVM_ON_UNIX)
 
 TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
Index: clang/lib/Basic/SourceManager.cpp
===================================================================
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -1250,23 +1250,18 @@
 
   const unsigned char *Buf = (const unsigned char *)Buffer->getBufferStart();
   const unsigned char *End = (const unsigned char *)Buffer->getBufferEnd();
+  const std::size_t BufLen = End - Buf;
   unsigned I = 0;
-  while (true) {
-    // Skip over the contents of the line.
-    while (Buf[I] != '\n' && Buf[I] != '\r' && Buf[I] != '\0')
-      ++I;
-
-    if (Buf[I] == '\n' || Buf[I] == '\r') {
+  while (I < BufLen) {
+    if (Buf[I] == '\n') {
+      LineOffsets.push_back(I + 1);
+    } else if (Buf[I] == '\r') {
       // If this is \r\n, skip both characters.
-      if (Buf[I] == '\r' && Buf[I+1] == '\n')
+      if (I + 1 < BufLen && Buf[I + 1] == '\n')
         ++I;
-      ++I;
-      LineOffsets.push_back(I);
-    } else {
-      // Otherwise, this is a NUL. If end of file, exit.
-      if (Buf+I == End) break;
-      ++I;
+      LineOffsets.push_back(I + 1);
     }
+    ++I;
   }
 
   // Copy the offsets into the FileInfo structure.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72409.237399.patch
Type: text/x-patch
Size: 2503 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200110/475aea57/attachment.bin>