Check special-case-list regex before insertion.

[hctim]

Summary

Checks that the supplied regex to SpecialCaseList::Matcher::insert(..) is non-empty.

Reported by OSS-fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3688

Verified that this fixes the provided assertion failure (built with {asan, fuzzer}):

mitchp@mitchp2:~/llvm-build/git-fuzz$ ninja llvm-special-case-list-fuzzer[12/12] Linking CXX executable bin/llvm-special-case-list-fuzzer
mitchp@mitchp2:~/llvm-build/git-fuzz$ bin/llvm-special-case-list-fuzzer ~/Downloads/clusterfuzz-testcase-6748633157337088 
INFO: Seed: 1697404507
INFO: Loaded 1 modules   (18581 inline 8-bit counters): 18581 [0x9e9f60, 0x9ee7f5), 
INFO: Loaded 1 PC tables (18581 PCs): 18581 [0x9ee7f8,0xa37148), 
bin/llvm-special-case-list-fuzzer: Running 1 inputs 1 time(s) each.
Running: /usr/local/google/home/mitchp/Downloads/clusterfuzz-testcase-6748633157337088
Executed /usr/local/google/home/mitchp/Downloads/clusterfuzz-testcase-6748633157337088 in 0 ms
***
*** NOTE: fuzzing was not performed, you have only
***       executed the target code on a fixed set of inputs.
***
mitchp@mitchp2:~/llvm-build/git-fuzz$

[vsk]

lib/Support/SpecialCaseList.cpp

40

CC'ing @vlad.tsyrklevich

You might prevent more bugs by having TrigramIndex::insert check for empty strings and return an Error. Moving the check there should also make it easier to add a unit test (see TrigramIndexTest.cpp).

[vlad.tsyrklevich]

Please add a test.

lib/Support/SpecialCaseList.cpp

40

In this case the exception is actually hit before we even get to the TrigramIndex--it's on insertion into Strings since the empty string is a literal. I'm not sure if TrigramIndex should try to check regexp validity. That seems like a complicated thing to do and TrigramIndex would only be able to look for the empty string. If this were actually to hit the regexp case we would have caught it with the isValid() check below.

[vsk]

lib/Support/SpecialCaseList.cpp

40

Sorry, I misread the backtrace and assumed the error occurred when inserting into Trigrams. I agree with what you've said.

It would still be useful to add a test along with this patch (SpecialCaseListTest.cpp):

@@ -67,6 +67,9 @@ TEST_F(SpecialCaseListTest, SectionRegexErrorHandling) {
 
   EXPECT_EQ(makeSpecialCaseList("[[]", Error), nullptr);
   EXPECT_TRUE(((StringRef)Error).startswith("malformed regex for section [: "));
+
+  EXPECT_EQ(makeSpecialCaseList("src:=", Error), nullptr);
+  EXPECT_TRUE(((StringRef)Error).endswith("Supplied regexp was blank"));
 }

[hctim]

• Added unit test to catch fuzz failure.

[vsk]

Yep, lgtm.

lib/Support/SpecialCaseList.cpp

 #include <system_error>
 #include <utility>
 
 #include <stdio.h>
 namespace llvm {
 
 bool SpecialCaseList::Matcher::insert(std::string Regexp,
                                       std::string &REError) {
+  if (Regexp.empty()) {
+    REError = "Supplied regexp was blank";
+    return false;
+  }
+
   if (Regex::isLiteralERE(Regexp)) {
     Strings.insert(Regexp);
     return true;
   }
   Trigrams.insert(Regexp);
 
   // Replace * with .*
   for (size_t pos = 0; (pos = Regexp.find('*', pos)) != std::string::npos;
        pos += strlen(".*")) {
     Regexp.replace(pos, strlen("*"), ".*");
   }
 
   // Check that the regexp is valid.