[compiler-rt] a7d4826 - [profile] Fix buffer overrun when parsing %c in filename string

[Vedant Kumar via llvm-commits]

Author: Vedant Kumar
Date: 2021-02-24T14:49:45-08:00
New Revision: a7d4826101aba8594bf5308c6a3e40c44608bca5

URL: https://github.com/llvm/llvm-project/commit/a7d4826101aba8594bf5308c6a3e40c44608bca5
DIFF: https://github.com/llvm/llvm-project/commit/a7d4826101aba8594bf5308c6a3e40c44608bca5.diff

LOG: [profile] Fix buffer overrun when parsing %c in filename string

Fix a buffer overrun that can occur when parsing '%c' at the end of a
filename pattern string.

rdar://74571261

Reviewed By: kastiglione

Differential Revision: https://reviews.llvm.org/D97239

Added: 
    compiler-rt/test/profile/ContinuousSyncMode/get-filename.c

Modified: 
    compiler-rt/lib/profile/InstrProfilingFile.c

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index 42ffdae82622..656df480ba2c 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -8,6 +8,7 @@
 
 #if !defined(__Fuchsia__)
 
+#include <assert.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -699,6 +700,13 @@ static unsigned getMergePoolSize(const char *FilenamePat, int *I) {
   return 0;
 }
 
+/* Assert that Idx does index past a string null terminator. Return the
+ * result of the check. */
+static int checkBounds(int Idx, int Strlen) {
+  assert(Idx <= Strlen && "Indexing past string null terminator");
+  return Idx <= Strlen;
+}
+
 /* Parses the pattern string \p FilenamePat and stores the result to
  * lprofcurFilename structure. */
 static int parseFilenamePattern(const char *FilenamePat,
@@ -707,6 +715,7 @@ static int parseFilenamePattern(const char *FilenamePat,
   char *PidChars = &lprofCurFilename.PidChars[0];
   char *Hostname = &lprofCurFilename.Hostname[0];
   int MergingEnabled = 0;
+  int FilenamePatLen = strlen(FilenamePat);
 
   /* Clean up cached prefix and filename.  */
   if (lprofCurFilename.ProfilePathPrefix)
@@ -725,9 +734,12 @@ static int parseFilenamePattern(const char *FilenamePat,
     lprofCurFilename.OwnsFilenamePat = 1;
   }
   /* Check the filename for "%p", which indicates a pid-substitution. */
-  for (I = 0; FilenamePat[I]; ++I)
+  for (I = 0; checkBounds(I, FilenamePatLen) && FilenamePat[I]; ++I) {
     if (FilenamePat[I] == '%') {
-      if (FilenamePat[++I] == 'p') {
+      ++I; /* Advance to the next character. */
+      if (!checkBounds(I, FilenamePatLen))
+        break;
+      if (FilenamePat[I] == 'p') {
         if (!NumPids++) {
           if (snprintf(PidChars, MAX_PID_SIZE, "%ld", (long)getpid()) <= 0) {
             PROF_WARN("Unable to get pid for filename pattern %s. Using the "
@@ -761,7 +773,6 @@ static int parseFilenamePattern(const char *FilenamePat,
 
         __llvm_profile_set_page_size(getpagesize());
         __llvm_profile_enable_continuous_mode();
-        I++; /* advance to 'c' */
       } else {
         unsigned MergePoolSize = getMergePoolSize(FilenamePat, &I);
         if (!MergePoolSize)
@@ -775,6 +786,7 @@ static int parseFilenamePattern(const char *FilenamePat,
         lprofCurFilename.MergePoolSize = MergePoolSize;
       }
     }
+  }
 
   lprofCurFilename.NumPids = NumPids;
   lprofCurFilename.NumHosts = NumHosts;

diff  --git a/compiler-rt/test/profile/ContinuousSyncMode/get-filename.c b/compiler-rt/test/profile/ContinuousSyncMode/get-filename.c
new file mode 100644
index 000000000000..d70e11991e18
--- /dev/null
+++ b/compiler-rt/test/profile/ContinuousSyncMode/get-filename.c
@@ -0,0 +1,32 @@
+// REQUIRES: darwin
+
+// RUN: %clang_pgogen -o %t.exe %s
+// RUN: env LLVM_PROFILE_FILE="%c%t.profraw" %run %t.exe %t.profraw
+// RUN: env LLVM_PROFILE_FILE="%t%c.profraw" %run %t.exe %t.profraw
+// RUN: env LLVM_PROFILE_FILE="%t.profraw%c" %run %t.exe %t.profraw
+
+#include <string.h>
+#include <stdio.h>
+
+extern int __llvm_profile_is_continuous_mode_enabled(void);
+extern const char *__llvm_profile_get_filename();
+extern void __llvm_profile_set_dumped(void);
+
+int main(int argc, char **argv) {
+  if (!__llvm_profile_is_continuous_mode_enabled())
+    return 1;
+
+  // Check that the filename is "%t.profraw", followed by a null terminator.
+  size_t n = strlen(argv[1]) + 1;
+  const char *Filename = __llvm_profile_get_filename();
+
+  for (int i = 0; i < n; ++i) {
+    if (Filename[i] != argv[1][i]) {
+      printf("Difference at: %d, Got: %c, Expected: %c\n", i, Filename[i], argv[1][i]);
+      printf("Got: %s\n", Filename);
+      printf("Expected: %s\n", argv[1]);
+      return 1;
+    }
+  }
+  return 0;
+}