[analyzer] CERT STR rule checkers: STR30-C

[Charusso]

Summary

This checker is implemented based on the following rule:
https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals

It warns on misusing the following functions: strpbrk(), strchr(),
strrchr(), strstr(), memchr().

[Charusso]

Examples generated by CodeChecker from the curl project:

str30-c.tar.gz1 MBDownload

[NoQ]

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

2109

Why does it matter whether the region is symbolic or not?

2110

Mmm, that's not a correct return value for these functions. These functions don't simply pass through their first argument.

2122

Why "heap"?

[Charusso]

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

2109

Hm, I think you are right, we could omit that if-statement. I wanted to specify to reuse conjured symbols.

2110

Yes, but we need some index here. It requires a NonLoc, so I just randomly picked the first index, but I like the idea of an unknown index. Would we like to introduce UnknownVal for indices?

2122

Well, a string which length is at least 16 characters long is going to be allocated on the heap. I have to conjure the string here to create its element.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

2110

Use the correct region but conjure the index.

2122

o.o

void foo() {
  // This string is 20 characters long
  // but it's clearly on the stack.
  char str[] = "12345678901234567890";
  // This one is therefore also on the stack.
  char *ptr = strchr(str, '0');
}

[Charusso]

• Try to conjure the index of the 'ElementRegion'.

[Charusso]

In order to bypass [1] the CK_LValueToRValue evalCast() we have to create en ElementRegion as a return-value of the problematic function call. In that case for a mythical reason we miss the fact the pointer is nullable. I have not figured out yet why, but tried to create an appropriate return-value using a VarRegion so we could refer to the underlying constant string's name.

[1] bypass: If I tried to evaluate the call by returning a pointer to the element it immediately wraps the SymbolicRegion into an ElementRegion, like with HeapSpaceRegions it is useful, but for us it is bad.

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

2122

Well, a string which length is at least 16 characters long is going to be allocated on the heap. I have to conjure the string here to create its element.

I really felt that the std::string should behave like the C-strings, but C-strings are on the stack whatever it takes, yes, my bad. Thanks for pointing that out!

clang/test/Analysis/cert/str30-c-notes.cpp

30

Needs to be an assumption.

[NoQ]

Let's separate CStringChecker improvements into a separate patch and have a separate set of tests for it.

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

2097–2098

Ok, so this whole thing is trying to evaluate strchr-like functions, right? I've no idea what it does but the problem is much more trivial that what you're trying to do here.

Branch 1:

1. Conjure the offset.
2. Add it to the original pointer (using evalBinOp).
3. Bind the result.

Branch 2:

1. Bind null.

And you probably should drop branch 2 completely because usually there's no indication that the character may in fact be missing in the string, and i don't want more null dereference false alarms. So it's just branch 1.

So the whole function should be 3 lines of code (maybe with a couple line breaks).

Well, maybe you should also handle the case where the original pointer is null (not sure if it's an immediate UB or not).

This could be improved by actually taking into account the contents of the string, but you don't seem to be trying to do this here.

2101–2103

So, like, StateNull is the state in which it is assumed that 0 is non-zero and State is the state in which it is assumed that 0 is zero?

I mean, apart from the naming flip-flop - i can tell you in advance that 0 is zero, it's not a matter of assumptions.

2127–2128

This is guaranteed to return the string region that you already have as the value of that expression.

[Charusso]

• Remove the modeling from CStringChecker.

[Charusso]

Thanks! I have felt that may I create a too complex symbolic value. Sorry for stealing your time.

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

2097–2098

I've no idea what it does

I wanted to represent the root string's VarDecl in the symbolic value so I could refer to it. I think it became too complex and printing out the root string's variable name does not worth that complexity. But here it is from str30-c-explain.cpp:

char *slash;
slash = strrchr(pathname, '/');

clang_analyzer_dump(slash);
// CHECK: &Element{pathname,conj_$1{long long, LC1, S{{[0-9]+}}, #1},char}

clang_analyzer_explain(slash);
// CHECK: pointer to element of type 'char' with index 'symbol of type 'long long' conjured at statement 'pathname'' of parameter 'pathname'

This could be improved by actually taking into account the contents of the string, but you don't seem to be trying to do this here.

At first the name of the string would be cool to print out and we need dataflow support to make it cool. Given that it is too complex I have reverted the modeling part. Also I wanted to create a simpler modeling with your modeling solution for my name-storing purposes, but I cannot.

2101–2103

I wanted to force out the state split of an unknown indexing: null and non-null but may it was too explicit and useless, yes.

2127–2128

Whoops. The idea was that to handle string regions explicitly and may calculate the returning index as well.

clang/test/Analysis/cert/str30-c-notes.cpp

30

Let us say it is non-null.

[Charusso]

• Refactor.

[Charusso]

In D71155#1854908, @NoQ wrote:

Let's separate CStringChecker improvements into a separate patch and have a separate set of tests for it.

I was thinking about creating tests, but I cannot figure out any better testing than a live checker that uses such features of CStringChecker. Also I have not found any real world issues with the checker, so I could not really test its real behavior.

[baloghadamsoftware]

clang/docs/analyzer/checkers.rst

1917

STR30-C is more general: Do not attempt to modify string literals. You should not check for these functions specifically, just model that they return string literals. However, you can also declare a string literal yourself. The checker should look for modifications of any string literals, whether returned by these functions or not.

clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp

263

I do not see V used in this function anywhere. We are not interested to what it is attempted to be modified. We are just interested that it is attempted to be modified.

clang/docs/analyzer/checkers.rst

 
 .. _alpha-security-cert-str-checkers:
 
 alpha.security.cert.str
 ^^^^^^^^^^^^^^^^^^^^^^^
 
 SEI CERT checkers of STR `C coding rules<https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152038>`_.
 
+.. _alpha-security-cert-str-30-c:
+
+alpha.security.cert.str.30.c
+""""""""""""""""""""""""""""
+
+SEI CERT checker of `STR30-C rule<https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals>`_.
+
+It warns on misusing the following functions: ``strpbrk()``, ``strchr()``,
+``strrchr()``, ``strstr()``, ``memchr()``.
+
 .. _alpha-security-cert-str-31-c:
 
 alpha.security.cert.str.31.c
 """"""""""""""""""""""""""""
 
 SEI CERT checker of `STR31-C rule<https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator>`_.
 
 It warns on misusing the following functions:

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

 
 let ParentPackage = STR in {
 
 def StrCheckerBase : Checker<"StrCheckerBase">,
   HelpText<"SEI CERT base checker of rules defined in STR">,
   Documentation<HasDocumentation>,
   Hidden;
 
+def Str30cChecker : Checker<"30.c">,
+  HelpText<"SEI CERT checker of rules defined in STR30-C">,
+  Dependencies<[StrCheckerBase]>,
+  Documentation<HasDocumentation>;
+
 def Str31cChecker : Checker<"31.c">,
   HelpText<"SEI CERT checker of rules defined in STR31-C">,
   Dependencies<[StrCheckerBase]>,
   Documentation<HasDocumentation>;
 
 def Str32cChecker : Checker<"32.c">,
   HelpText<"SEI CERT checker of rules defined in STR32-C">,
   Dependencies<[StrCheckerBase]>,

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

public:
 
   typedef void (CStringChecker::*FnCheck)(CheckerContext &,
                                           const CallExpr *) const;
   CallDescriptionMap<FnCheck> Callbacks = {
       {{CDF_MaybeBuiltin, "memcpy", 3}, &CStringChecker::evalMemcpy},
       {{CDF_MaybeBuiltin, "mempcpy", 3}, &CStringChecker::evalMempcpy},
       {{CDF_MaybeBuiltin, "memcmp", 3}, &CStringChecker::evalMemcmp},
       {{CDF_MaybeBuiltin, "memmove", 3}, &CStringChecker::evalMemmove},
+      {{CDF_MaybeBuiltin, "memchr", 3}, &CStringChecker::evalMemchr},
       {{CDF_MaybeBuiltin, "memset", 3}, &CStringChecker::evalMemset},
       {{CDF_MaybeBuiltin, "explicit_memset", 3}, &CStringChecker::evalMemset},
       {{CDF_MaybeBuiltin, "strcpy", 2}, &CStringChecker::evalStrcpy},
       {{CDF_MaybeBuiltin, "strncpy", 3}, &CStringChecker::evalStrncpy},
       {{CDF_MaybeBuiltin, "stpcpy", 2}, &CStringChecker::evalStpcpy},
       {{CDF_MaybeBuiltin, "strlcpy", 3}, &CStringChecker::evalStrlcpy},
       {{CDF_MaybeBuiltin, "strcat", 2}, &CStringChecker::evalStrcat},
       {{CDF_MaybeBuiltin, "strncat", 3}, &CStringChecker::evalStrncat},
       {{CDF_MaybeBuiltin, "strlcat", 3}, &CStringChecker::evalStrlcat},
       {{CDF_MaybeBuiltin, "strlen", 1}, &CStringChecker::evalstrLength},
       {{CDF_MaybeBuiltin, "strnlen", 2}, &CStringChecker::evalstrnLength},
       {{CDF_MaybeBuiltin, "strcmp", 2}, &CStringChecker::evalStrcmp},
       {{CDF_MaybeBuiltin, "strncmp", 3}, &CStringChecker::evalStrncmp},
       {{CDF_MaybeBuiltin, "strcasecmp", 2}, &CStringChecker::evalStrcasecmp},
       {{CDF_MaybeBuiltin, "strncasecmp", 3}, &CStringChecker::evalStrncasecmp},
       {{CDF_MaybeBuiltin, "strsep", 2}, &CStringChecker::evalStrsep},
+      {{CDF_MaybeBuiltin, "strpbrk", 2}, &CStringChecker::evalStrpbrk},
+      {{CDF_MaybeBuiltin, "strchr", 2}, &CStringChecker::evalStrchr},
+      {{CDF_MaybeBuiltin, "strrchr", 2}, &CStringChecker::evalStrrchr},
+      {{CDF_MaybeBuiltin, "strstr", 2}, &CStringChecker::evalStrstr},
       {{CDF_MaybeBuiltin, "bcopy", 3}, &CStringChecker::evalBcopy},
       {{CDF_MaybeBuiltin, "bcmp", 3}, &CStringChecker::evalMemcmp},
       {{CDF_MaybeBuiltin, "bzero", 2}, &CStringChecker::evalBzero},
       {{CDF_MaybeBuiltin, "explicit_bzero", 2}, &CStringChecker::evalBzero},
   };
 
   // These require a bit of special handling.
   CallDescription StdCopy{{"std", "copy"}, 3},
public:
   void evalStrncasecmp(CheckerContext &C, const CallExpr *CE) const;
   void evalStrcmpCommon(CheckerContext &C,
                         const CallExpr *CE,
                         bool isBounded = false,
                         bool ignoreCase = false) const;
 
   void evalStrsep(CheckerContext &C, const CallExpr *CE) const;
 
+  void evalCharPtrCommon(CheckerContext &C, const CallExpr *CE) const;
+  void evalMemchr(CheckerContext &C, const CallExpr *CE) const;
+  void evalStrchr(CheckerContext &C, const CallExpr *CE) const;
+  void evalStrrchr(CheckerContext &C, const CallExpr *CE) const;
+  void evalStrstr(CheckerContext &C, const CallExpr *CE) const;
+  void evalStrpbrk(CheckerContext &C, const CallExpr *CE) const;
+
   void evalStdCopy(CheckerContext &C, const CallExpr *CE) const;
   void evalStdCopyBackward(CheckerContext &C, const CallExpr *CE) const;
   void evalStdCopyCommon(CheckerContext &C, const CallExpr *CE) const;
   void evalMemset(CheckerContext &C, const CallExpr *CE) const;
   void evalBzero(CheckerContext &C, const CallExpr *CE) const;
 
   // Utility methods
   std::pair<ProgramStateRef , ProgramStateRef >
if (Optional<Loc> SearchStrLoc = SearchStrVal.getAs<Loc>()) {
     Result = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
   }
 
   // Set the return value, and finish.
   State = State->BindExpr(CE, LCtx, Result);
   C.addTransition(State);
 }
 
+void CStringChecker::evalCharPtrCommon(CheckerContext &C,
+                                       const CallExpr *CE) const {
+  CurrentFunctionDescription = "char pointer returning function";
+
+  ProgramStateRef State = C.getState();
+  SValBuilder &SVB = C.getSValBuilder();
+  ASTContext &Ctx = C.getASTContext();
+  const LocationContext *LCtx = C.getLocationContext();
+
+  // Check whether we already conjured a symbol for the pointer.
+  SVal SrcV = C.getSVal(CE->getArg(0));
+  if (const MemRegion *MR = SrcV.getAsRegion()) {
+    if (const auto *SR = MR->getBaseRegion()->getAs<SymbolicRegion>()) {
+      State = State->BindExpr(CE, LCtx, SrcV);
+      C.addTransition(State);
+      return;
+    }
+  }
+
+  // Conjure a symbol to represent the pointer.
+  NonLoc RandomIdx = SVB.makeArrayIndex(0);
+  QualType Ty = CE->getArg(0)->getType();
+  if (Ty->isPointerType())
+    Ty = Ty->getPointeeType();
+
+  SVal ConjuredV = SVB.getConjuredHeapSymbolVal(CE, LCtx, C.blockCount());
+  SVal ResultV = loc::MemRegionVal(SVB.getRegionManager().getElementRegion(
+      Ty, RandomIdx, cast<SubRegion>(ConjuredV.getAsRegion()), Ctx));
+
+  State = State->BindExpr(CE, LCtx, ResultV);
+  C.addTransition(State);
+}
+
+void CStringChecker::evalMemchr(CheckerContext &C, const CallExpr *CE) const {
+  // void *memchr(const void *buffer, int c, size_t count);
+  const Expr *BufferExpr = CE->getArg(0);
+  QualType Ty = BufferExpr->getType().getUnqualifiedType();
+  if (Ty->isPointerType())
+    Ty = Ty->getPointeeType();
+
+  if (Ty->isAnyCharacterType())
+    evalCharPtrCommon(C, CE);
+}
+
+void CStringChecker::evalStrchr(CheckerContext &C, const CallExpr *CE) const {
+  // char *strchr(const char *str, int c);
+  evalCharPtrCommon(C, CE);
+}
+
+void CStringChecker::evalStrrchr(CheckerContext &C, const CallExpr *CE) const {
+  // char *strrchr(const char *str, int c);
+  evalCharPtrCommon(C, CE);
+}
+
+void CStringChecker::evalStrstr(CheckerContext &C, const CallExpr *CE) const {
+  // char *strstr(const char *str, const char *strSearch);
+  evalCharPtrCommon(C, CE);
+}
+
+void CStringChecker::evalStrpbrk(CheckerContext &C, const CallExpr *CE) const {
+  // char *strpbrk(const char *str, const char *strCharSet);
+  evalCharPtrCommon(C, CE);
+}
+
 // These should probably be moved into a C++ standard library checker.
 void CStringChecker::evalStdCopy(CheckerContext &C, const CallExpr *CE) const {
   evalStdCopyCommon(C, CE);
 }
 
 void CStringChecker::evalStdCopyBackward(CheckerContext &C,
     const CallExpr *CE) const {
   evalStdCopyCommon(C, CE);

clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp

 //===- StrCheckerBase - SEI CERT rules checker defined in STR ---*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines StrCheckerBase which tries to find the SEI CERT string
 //  handler function issues.
 //  The rules can be found in section 'Rule 07. Characters and Strings (STR)':
 //  https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152038
 //
 //  This checker is a base checker which consist of the following checkers:
+//  - '30.c'
+//  https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals
+//
 //  - '31.c'
 //  https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator
 //
 //  - '32.c'
 //  https://wiki.sei.cmu.edu/confluence/display/c/STR32-C.+Do+not+pass+a+non-null-terminated+character+sequence+to+a+library+function+that+expects+a+string
 //
 //===----------------------------------------------------------------------===//
 
CallContext(Optional<unsigned> DestinationPos,
         LengthPos(LengthPos) {}
 
   Optional<unsigned> DestinationPos;
   Optional<unsigned> SourcePos;
   Optional<unsigned> LengthPos;
 };
 
 class StrCheckerBase : public Checker<check::PostCall, check::Bind> {
-  using StrCheck = std::function<void(const StrCheckerBase *, const CallEvent &,
-                                      const CallContext &, CheckerContext &)>;
-
 public:
   // We report a note when any of the calls in 'CDM' is being used because
   // they can cause a not null-terminated string. In this case we store the
   // string is being not null-terminated in the 'NullTerminationMap'.
   //
   // We report an error when the not null-terminated string is passed to a
   // function call. Assume that the non-'CDM' calls could read the string.
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
 
   // We want to catch the hand-written null-termination of strings to prevent
   // false positives. For example: 'c_string[length] = '\0';'. In this case we
   // store the string is being null-terminated in the 'NullTerminationMap'.
   void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
 
+  // STR30-C.
+  void checkMemchr(const CallEvent &Call, const CallContext &CallC,
+                  CheckerContext &C) const;
+  void checkStrchr(const CallEvent &Call, const CallContext &CallC,
+                  CheckerContext &C) const;
+  void checkStrrchr(const CallEvent &Call, const CallContext &CallC,
+                   CheckerContext &C) const;
+  void checkStrstr(const CallEvent &Call, const CallContext &CallC,
+                  CheckerContext &C) const;
+  void checkStrpbrk(const CallEvent &Call, const CallContext &CallC,
+                   CheckerContext &C) const;
+
+  // STR31-C.
   void checkGets(const CallEvent &Call, const CallContext &CallC,
-                 CheckerContext &C) const;
   void checkSprintf(const CallEvent &Call, const CallContext &CallC,
-                    CheckerContext &C) const;
   void checkFscanf(const CallEvent &Call, const CallContext &CallC,
-                   CheckerContext &C) const;
   void checkStrcpy(const CallEvent &Call, const CallContext &CallC,
-                   CheckerContext &C) const;
+
+  // STR32-C.
   void checkStrncpy(const CallEvent &Call, const CallContext &CallC,
-                    CheckerContext &C) const;
 
   std::unique_ptr<BugType> BT;
 
-  bool EnableStr31cChecker = false, EnableStr32cChecker = false;
+  bool EnableStr30cChecker = false, EnableStr31cChecker = false,
+       EnableStr32cChecker = false;
 
 private:
+  using StrCheck = std::function<void(const StrCheckerBase *, const CallEvent &,
+                                      const CallContext &, CheckerContext &)>;
+
   const CallDescriptionMap<std::pair<StrCheck, CallContext>> CDM = {
+      // The following checks STR30-C rules.
+      // void *memchr(const void *buffer, int c, size_t count);
+      {{"memchr", 3}, {&StrCheckerBase::checkMemchr, {None, 0}}},
+      // char *strchr(const char *str, int c);
+      {{"strchr", 2}, {&StrCheckerBase::checkStrchr, {None, 0}}},
+      // char *strrchr(const char *str, int c);
+      {{"strrchr", 2}, {&StrCheckerBase::checkStrrchr, {None, 0}}},
+      // char *strstr(const char *str, const char *strSearch);
+      {{"strstr", 2}, {&StrCheckerBase::checkStrstr, {None, 0}}},
+      // char *strpbrk(const char *str, const char *strCharSet);
+      {{"strpbrk", 2}, {&StrCheckerBase::checkStrpbrk, {None, 0}}},
+
       // The following checks STR31-C rules.
       // char *gets(char *dest);
       {{"gets", 1}, {&StrCheckerBase::checkGets, {0}}},
       // int sprintf(char *dest, const char *format, ... [const char *source]);
       {{"sprintf", 3, 2}, {&StrCheckerBase::checkSprintf, {0}}},
       // int fscanf(FILE *stream, const char *format, ... [char *dest]);
       {{"fscanf", 3, 2}, {&StrCheckerBase::checkFscanf, {2}}},
       // char *strcpy(char *dest, const char *src);
       {{"strcpy", 2}, {&StrCheckerBase::checkStrcpy, {0, 1}}},
 
       // The following checks STR32-C rules.
       // char *strncpy(char *dest, const char *src, size_t count)
       {{"strncpy", 3}, {&StrCheckerBase::checkStrncpy, {0, 1, 2}}}};
 };
 } // namespace
 
 // It stores whether the string is null-terminated.
 REGISTER_MAP_WITH_PROGRAMSTATE(NullTerminationMap, const MemRegion *, bool)
 
 // It stores the allocations which we emit notes on.
 REGISTER_LIST_WITH_PROGRAMSTATE(AllocationList, const MemRegion *)
 
+// It stores the strings which we have pointers to them.
+REGISTER_LIST_WITH_PROGRAMSTATE(PointerList, const MemRegion *)
+
 //===----------------------------------------------------------------------===//
 // Helper functions.
 //===----------------------------------------------------------------------===//
 
 // Returns a string representation of \p E.
 static std::string exprToStr(const Expr *E, CheckerContext &C) {
   assert(E);
 
static bool isWrongAllocation(const CallEvent &Call, const CallContext &CallC,
   SVal SrcV = Call.getArgSVal(*CallC.SourcePos);
   return isWrongAllocation(DestV, SrcV, C);
 }
 
 //===----------------------------------------------------------------------===//
 // Evaluating problematic function calls.
 //===----------------------------------------------------------------------===//
 
+static void checkCharPtr(const CallEvent &Call, const CallContext &CallC,
+                        CheckerContext &C) {
+  ProgramStateRef State = C.getState();
+  SVal ReturnV = Call.getReturnValue();
+  const MemRegion *MR = getRegion(ReturnV);
+  if (!MR)
+    return;
+
+  // Check for constant source string.
+  const Expr *SrcArg = Call.getArgExpr(*CallC.SourcePos);
+  QualType Ty = SrcArg->getType();
+  if (Ty->isPointerType())
+    Ty = Ty->getPointeeType();
+  if (!Ty.isConstQualified())
+    return;
+
+  std::string CallName = Call.getCalleeIdentifier()->getName();
+  std::string ArgName = "first argument";
+  if (const auto *SR = MR->getAs<SymbolicRegion>()) {
+    if (const auto *SC = dyn_cast<SymbolConjured>(SR->getSymbol())) {
+      if (Optional<std::string> ArgNameTmp =
+              getName(cast<CallExpr>(SC->getStmt())->getArg(0), C)) {
+        ArgName = *ArgNameTmp;
+      }
+    }
+  }
+
+  const NoteTag *Tag = C.getNoteTag(
+      [=]() -> std::string {
+        SmallString<128> Msg;
+        llvm::raw_svector_ostream Out(Msg);
+        Out << '\'' << CallName << "' returns a pointer to the constant "
+            << ArgName;
+
+        return Out.str();
+      },
+      /*IsPrunable=*/false);
+
+  State = State->add<PointerList>(MR);
+  C.addTransition(State, C.getPredecessor(), Tag);
+}
+
+void StrCheckerBase::checkMemchr(const CallEvent &Call, const CallContext &CallC,
+                                CheckerContext &C) const {
+  if (EnableStr30cChecker)
+    if (const MemRegion *MR = Call.getArgSVal(0).getAsRegion())
+      if (const auto *ER = MR->getAs<ElementRegion>())
+        if (ER->getValueType()->isAnyCharacterType())
+          checkCharPtr(Call, CallC, C);
+}
+
+void StrCheckerBase::checkStrchr(const CallEvent &Call, const CallContext &CallC,
+                                CheckerContext &C) const {
+  if (EnableStr30cChecker)
+    checkCharPtr(Call, CallC, C);
+}
+void StrCheckerBase::checkStrrchr(const CallEvent &Call,
+                                 const CallContext &CallC,
+                                 CheckerContext &C) const {
+  if (EnableStr30cChecker)
+    checkCharPtr(Call, CallC, C);
+}
+
+void StrCheckerBase::checkStrstr(const CallEvent &Call, const CallContext &CallC,
+                                CheckerContext &C) const {
+  if (EnableStr30cChecker)
+    checkCharPtr(Call, CallC, C);
+}
+
+void StrCheckerBase::checkStrpbrk(const CallEvent &Call,
+                                 const CallContext &CallC,
+                                 CheckerContext &C) const {
+  if (EnableStr30cChecker)
+    checkCharPtr(Call, CallC, C);
+}
+
 void StrCheckerBase::checkGets(const CallEvent &Call, const CallContext &CallC,
-                               CheckerContext &C) const {
   if (EnableStr31cChecker)
     createOverflowNote(Call, CallC, C);
 }
 
 void StrCheckerBase::checkSprintf(const CallEvent &Call,
                                   const CallContext &CallC,
                                   CheckerContext &C) const {
   if (EnableStr31cChecker)
static bool checkAllocation(SVal L, SVal V, const Stmt *S, CheckerContext &C,
 
   // Mark the string could be not null-terminated.
   State = State->set<NullTerminationMap>(NonLocMR, false);
   State = State->add<AllocationList>(NonLocMR);
   C.addTransition(State, C.getPredecessor(), Tag);
   return true;
 }
 
+static bool checkConstModify(SVal L, SVal V, const Stmt *S, CheckerContext &C,
+                             BugType &BT) {
+  const MemRegion *MR = getRegion(L);
+  if (!MR)
+    return false;
+
+  if (!C.getState()->contains<PointerList>(MR))
+    return false;
+
+  // Check for simple bindings.
+  const auto *BO = dyn_cast<BinaryOperator>(S);
+  if (!BO)
+    return false;
+
+  SmallString<128> Msg;
+  llvm::raw_svector_ostream Out(Msg);
+
+  std::string Name = "the dereferenced pointer";
+  if (Optional<std::string> NameTmp = getName(BO->getLHS(), C))
+    Name = *NameTmp;
+
+  Out << Name << " is pointing to a constant string";
+
+  auto Report = std::make_unique<PathSensitiveBugReport>(BT, Out.str(),
+                                                         C.generateErrorNode());
+
+  // Track the allocation.
+  bugreporter::trackExpressionValue(C.getPredecessor(), BO->getLHS(), *Report);
+
+  C.emitReport(std::move(Report));
+  return true;
+}
+
 void StrCheckerBase::checkBind(SVal L, SVal V, const Stmt *S,
                                CheckerContext &C) const {
+  if (EnableStr30cChecker)
+    if (checkConstModify(L, V, S, C, *BT))
+      return;
+
   if (EnableStr32cChecker)
     if (checkAllocation(L, V, S, C, *BT))
       return;
 
   ProgramStateRef State = C.getState();
   bool IsNullTermination = false;
   if (const llvm::APInt *Int = C.getSValBuilder().getKnownValue(State, V))
     IsNullTermination = Int->isNullValue();

 #define REGISTER_CHECKER(Name)                                                 \
   void ento::register##Name(CheckerManager &Mgr) {                             \
     auto *Checker = Mgr.getChecker<StrCheckerBase>();                          \
     Checker->Enable##Name = true;                                              \
   }                                                                            \
                                                                                \
   bool ento::shouldRegister##Name(const LangOptions &LO) { return true; }
 
+REGISTER_CHECKER(Str30cChecker)
 REGISTER_CHECKER(Str31cChecker)
 REGISTER_CHECKER(Str32cChecker)

clang/test/Analysis/cert/str30-c-notes.cpp

+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,alpha.security.cert.str.30.c \
+// RUN:  -analyzer-output=text -verify %s
+
+// See the examples on the page of STR30-C:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals
+
+#include "../Inputs/system-header-simulator.h"
+
+#define EOF -1
+typedef __SIZE_TYPE__ size_t;
+
+void free(void *memblock);
+void *malloc(size_t size);
+
+char *strrchr(const char *str, int c);
+int puts(const char *str);
+
+namespace test_strrchr_bad {
+const char *get_dirname(const char *pathname) {
+  char *slash;
+  slash = strrchr(pathname, '/');
+  // expected-note@-1 {{'strrchr' returns a pointer to the constant 'pathname'}}
+  
+  slash = strrchr(slash, '/');
+  // expected-note@-1 {{'strrchr' returns a pointer to the constant 'pathname'}}
+
+  if (slash) {
+    // expected-note@-1 {{Assuming 'slash' is non-null}}
+    // expected-note@-2 {{Taking true branch}}
+
+    *slash = '\0'; /* Undefined behavior */
+    // expected-note@-1 {{'slash' is pointing to a constant string}}
+    // expected-warning@-2 {{'slash' is pointing to a constant string}}
+  }
+  return pathname;
+}
+
+int main(void) {
+  puts(get_dirname(__FILE__));
+  // expected-note@-1 {{Calling 'get_dirname'}}
+  return 0;
+}
+} // namespace test_strrchr_bad

clang/test/Analysis/cert/str30-c.cpp

+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,alpha.security.cert.str.30.c \
+// RUN:  -verify %s
+
+// See the examples on the page of STR30-C:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals
+
+#include "../Inputs/system-header-simulator.h"
+
+#define EOF -1
+typedef __SIZE_TYPE__ size_t;
+typedef __typeof__((char*)0-(char*)0) ptrdiff_t;
+
+void free(void *memblock);
+void *malloc(size_t size);
+
+char *strrchr(const char *str, int c);
+int puts(const char *str);
+
+namespace test_strrchr_bad {
+const char *get_dirname(const char *pathname) {
+  char *slash;
+  slash = strrchr(pathname, '/');
+  if (slash) {
+    *slash = '\0'; /* Undefined behavior */
+    // expected-warning@-1 {{'slash' is pointing to a constant string}}
+  }
+  return pathname;
+}
+
+int main(void) {
+  puts(get_dirname(__FILE__));
+  return 0;
+}
+} // namespace test_strrchr_bad
+
+namespace test_strrchr_good {
+char *get_dirname(const char *pathname, char *dirname, size_t size) {
+  const char *slash;
+  slash = strrchr(pathname, '/');
+  if (slash) {
+    ptrdiff_t slash_idx = slash - pathname;
+    if ((size_t)slash_idx < size) {
+      memcpy(dirname, pathname, slash_idx);
+      dirname[slash_idx] = '\0';     
+      return dirname;
+    }
+  }
+  return 0;
+}
+
+int main(void) {
+  char dirname[260];
+  if (get_dirname(__FILE__, dirname, sizeof(dirname))) {
+    puts(dirname);
+  }
+  return 0;
+}
+} // namespace test_strrchr_good