[clang] [clang][analyzer] Add checker 'Security.SetgidSetuidOrder'. (PR #91445)

[Donát Nagy via cfe-commits]

================
@@ -0,0 +1,197 @@
+//===-- SetgidSetuidOrderChecker.cpp - check privilege revocation calls ---===//
+//
+// 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 a checker to detect possible reversed order of privilege
+//  revocations when 'setgid' and 'setuid' is used.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class SetgidSetuidOrderChecker
+    : public Checker<check::PostCall, check::DeadSymbols, eval::Assume> {
+  const BugType BT_WrongRevocationOrder{
+      this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{{"setuid"}, 1};
+  const CallDescription SetgidDesc{{"setgid"}, 1};
+  const CallDescription SeteuidDesc{{"seteuid"}, 1};
+  const CallDescription SetegidDesc{{"setegid"}, 1};
+  const CallDescription SetreuidDesc{{"setreuid"}, 2};
+  const CallDescription SetregidDesc{{"setregid"}, 2};
+  const CallDescription SetresuidDesc{{"setresuid"}, 3};
+  const CallDescription SetresgidDesc{{"setresgid"}, 3};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+                             bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent &Call,
+                                CheckerContext &C) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent &Call,
+                                CheckerContext &C) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent &Call,
+                               CheckerContext &C) const;
+  /// Check if a function like \c getuid or \c getgid is called directly from
+  /// the first argument of function called from \a Call.
+  bool isFunctionCalledInArg(llvm::StringRef FnName,
+                             const CallEvent &Call) const;
+  void emitReport(ProgramStateRef State, CheckerContext &C) const;
+};
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+} // end anonymous namespace
+
+/// Store if there was a call to 'setuid(getuid())' or 'setgid(getgid())' not
+/// followed by other different privilege-change functions.
+/// If the value \c Setuid is stored and a 'setgid(getgid())' call is found we
+/// have found the bug to be reported. Value \c Setgid is used too to prevent
+/// warnings at a setgid-setuid-setgid sequence.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(LastSetPrivilegeCall, SetPrivilegeFunctionKind)
+/// Store the symbol value of the last 'setuid(getuid())' call. This is used to
+/// detect if the result is compared to -1 and avoid warnings on that branch
+/// (which is the failure branch of the call).
+REGISTER_TRAIT_WITH_PROGRAMSTATE(LastSetuidCallSVal, SymbolRef)
+
+void SetgidSetuidOrderChecker::checkPostCall(const CallEvent &Call,
+                                             CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  if (SetuidDesc.matches(Call)) {
+    State = processSetuid(State, Call, C);
+  } else if (SetgidDesc.matches(Call)) {
+    State = processSetgid(State, Call, C);
+  } else if (matchesAny(Call, SeteuidDesc, SetegidDesc, SetreuidDesc,
+                        SetregidDesc, SetresuidDesc, SetresgidDesc)) {
----------------
NagyDonat wrote:

`matchesAny` is a nice tool, but it would be even better to use a `CallDescriptionSet` and its `contains` method, because then you wouldn't need to introduce individual variable names for these call descriptions. 

https://github.com/llvm/llvm-project/pull/91445