[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

[Balazs Benics via cfe-commits]

https://github.com/steakhal approved this pull request.

LGTM, thanks!

https://github.com/llvm/llvm-project/pull/91879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Use ArrayRef for input parameters (PR #93203)

[Balazs Benics via cfe-commits]

https://github.com/steakhal created 
https://github.com/llvm/llvm-project/pull/93203

Fixes #79684

>From ef65ed8c193c43c1914dc39bf1cd48da83872fc5 Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Thu, 23 May 2024 10:56:33 +0200
Subject: [PATCH] [analyzer][NFC] Use ArrayRef for input parameters

Fixes #79684
---
 .../StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp| 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 845a5f9b390dc..8f4bd17afc858 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -672,7 +672,7 @@ class StdLibraryFunctionsChecker
 StringRef getNote() const { return Note; }
   };
 
-  using ArgTypes = std::vector>;
+  using ArgTypes = ArrayRef>;
   using RetType = std::optional;
 
   // A placeholder type, we use it whenever we do not care about the concrete
@@ -1746,7 +1746,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 }
 // Add the same summary for different names with the Signature explicitly
 // given.
-void operator()(std::vector Names, Signature Sign, Summary Sum) 
{
+void operator()(ArrayRef Names, Signature Sign, Summary Sum) {
   for (StringRef Name : Names)
 operator()(Name, Sign, Sum);
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST] Fix end location of DeclarationNameInfo on instantiated methods (PR #92654)

[Balazs Benics via cfe-commits]

https://github.com/steakhal closed 
https://github.com/llvm/llvm-project/pull/92654
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST] Fix end location of DeclarationNameInfo on instantiated methods (PR #92654)

[Balazs Benics via cfe-commits]

https://github.com/steakhal updated 
https://github.com/llvm/llvm-project/pull/92654

>From 3f3c98a55a6d89ddb05085c41d1fffad331595ce Mon Sep 17 00:00:00 2001
From: Alejandro _lvarez Ayll_n 
Date: Sat, 18 May 2024 16:53:33 +0200
Subject: [PATCH] [clang][AST] Fix end location of DeclarationNameInfo on
 instantiated methods

Fixes #71161

[D64087](https://reviews.llvm.org/D64087) updated some locations of the
instantiated method but forgot `DNLoc`.

`FunctionDecl::getNameInfo()` constructs a `DeclarationNameInfo` using
`Decl::Loc` as the beginning of the declaration name, and
`FunctionDecl::DNLoc` to compute the end of the declaration name. The
former was updated, but the latter was not, so
`DeclarationName::getSourceRange()` would return a range where the end
of the declaration name could come before its beginning.

Patch by Alejandro Alvarez Ayllon
Co-authored-by: steakhal

CPP-5166
---
 clang/docs/ReleaseNotes.rst   |  1 +
 clang/include/clang/AST/Decl.h|  2 ++
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  1 +
 clang/unittests/AST/DeclTest.cpp  | 31 +++
 4 files changed, 35 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2899bc5ed35ad..93b6ba59ecf9f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -766,6 +766,7 @@ Miscellaneous Bug Fixes
 
 - Fixed an infinite recursion in ASTImporter, on return type declared inside
   body of C++11 lambda without trailing return (#GH68775).
+- Fixed declaration name source location of instantiated function definitions 
(GH71161).
 
 Miscellaneous Clang Crashes Fixed
 ^
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 5e485ccb85a13..7fd80b90d1033 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2188,6 +2188,8 @@ class FunctionDecl : public DeclaratorDecl,
 
   void setRangeEnd(SourceLocation E) { EndRangeLoc = E; }
 
+  void setDeclarationNameLoc(DeclarationNameLoc L) { DNLoc = L; }
+
   /// Returns the location of the ellipsis of a variadic function.
   SourceLocation getEllipsisLoc() const {
 const auto *FPT = getType()->getAs();
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 09812946bd383..bb49aae2cb666 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5055,6 +5055,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation 
PointOfInstantiation,
   Function->setLocation(PatternDecl->getLocation());
   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
   Function->setRangeEnd(PatternDecl->getEndLoc());
+  Function->setDeclarationNameLoc(PatternDecl->getNameInfo().getInfo());
 
   EnterExpressionEvaluationContext EvalContext(
   *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp
index 2530ce74eb6a3..16aa2b50b7a06 100644
--- a/clang/unittests/AST/DeclTest.cpp
+++ b/clang/unittests/AST/DeclTest.cpp
@@ -545,3 +545,34 @@ TEST(Decl, TemplateArgumentDefaulted) {
   EXPECT_TRUE(ArgList.get(2).getIsDefaulted());
   EXPECT_TRUE(ArgList.get(3).getIsDefaulted());
 }
+
+TEST(Decl, CXXDestructorDeclsShouldHaveWellFormedNameInfoRanges) {
+  // GH71161
+  llvm::Annotations Code(R"cpp(
+template  struct Resource {
+  ~Resource(); // 1
+};
+template 
+Resource::~Resource() {} // 2,3
+
+void instantiate_template() {
+  Resource x;
+}
+)cpp");
+
+  auto AST = tooling::buildASTFromCode(Code.code());
+  ASTContext  = AST->getASTContext();
+
+  const auto  = Ctx.getSourceManager();
+  auto GetNameInfoRange = [](const BoundNodes ) {
+const auto *D = Match.getNodeAs("dtor");
+return D->getNameInfo().getSourceRange().printToString(SM);
+  };
+
+  auto Matches = match(findAll(cxxDestructorDecl().bind("dtor")),
+   *Ctx.getTranslationUnitDecl(), Ctx);
+  ASSERT_EQ(Matches.size(), 3U);
+  EXPECT_EQ(GetNameInfoRange(Matches[0]), "");
+  EXPECT_EQ(GetNameInfoRange(Matches[1]), "");
+  EXPECT_EQ(GetNameInfoRange(Matches[2]), "");
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.security.cert.pos.34c into security.PutenvWithAuto (PR #92424)

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


steakhal wrote:

Make sure you adjust/sync the commit title, content and the PR title before 
merging.

https://github.com/llvm/llvm-project/pull/92424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST] Fix end location of DeclarationNameInfo on instantiated methods (PR #92654)

[Balazs Benics via cfe-commits]

steakhal wrote:

> nit: add a note in `clang/docs/ReleaseNotes.rst`

Thanks. Added. Let me know if it's in the right section. @hokein

https://github.com/llvm/llvm-project/pull/92654
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST] Fix end location of DeclarationNameInfo on instantiated methods (PR #92654)

[Balazs Benics via cfe-commits]

https://github.com/steakhal updated 
https://github.com/llvm/llvm-project/pull/92654

>From 58ca4d9be1dbc43ad40b6e26b8a5d79e20be8d93 Mon Sep 17 00:00:00 2001
From: Alejandro _lvarez Ayll_n 
Date: Sat, 18 May 2024 16:53:33 +0200
Subject: [PATCH 1/2] [clang][AST] Fix end location of DeclarationNameInfo on
 instantiated methods

Fixes #71161

[D64087](https://reviews.llvm.org/D64087) updated some locations of the
instantiated method but forgot `DNLoc`.

`FunctionDecl::getNameInfo()` constructs a `DeclarationNameInfo` using
`Decl::Loc` as the beginning of the declaration name, and
`FunctionDecl::DNLoc` to compute the end of the declaration name. The
former was updated, but the latter was not, so
`DeclarationName::getSourceRange()` would return a range where the end
of the declaration name could come before its beginning.

Patch by Alejandro Alvarez Ayllon
Co-authored-by: steakhal

CPP-5166
---
 clang/include/clang/AST/Decl.h|  2 ++
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  1 +
 clang/unittests/AST/DeclTest.cpp  | 31 +++
 3 files changed, 34 insertions(+)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 5e485ccb85a13..7fd80b90d1033 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2188,6 +2188,8 @@ class FunctionDecl : public DeclaratorDecl,
 
   void setRangeEnd(SourceLocation E) { EndRangeLoc = E; }
 
+  void setDeclarationNameLoc(DeclarationNameLoc L) { DNLoc = L; }
+
   /// Returns the location of the ellipsis of a variadic function.
   SourceLocation getEllipsisLoc() const {
 const auto *FPT = getType()->getAs();
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 381d79b2fcd46..6d736d0771eac 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5055,6 +5055,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation 
PointOfInstantiation,
   Function->setLocation(PatternDecl->getLocation());
   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
   Function->setRangeEnd(PatternDecl->getEndLoc());
+  Function->setDeclarationNameLoc(PatternDecl->getNameInfo().getInfo());
 
   EnterExpressionEvaluationContext EvalContext(
   *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp
index 2530ce74eb6a3..16aa2b50b7a06 100644
--- a/clang/unittests/AST/DeclTest.cpp
+++ b/clang/unittests/AST/DeclTest.cpp
@@ -545,3 +545,34 @@ TEST(Decl, TemplateArgumentDefaulted) {
   EXPECT_TRUE(ArgList.get(2).getIsDefaulted());
   EXPECT_TRUE(ArgList.get(3).getIsDefaulted());
 }
+
+TEST(Decl, CXXDestructorDeclsShouldHaveWellFormedNameInfoRanges) {
+  // GH71161
+  llvm::Annotations Code(R"cpp(
+template  struct Resource {
+  ~Resource(); // 1
+};
+template 
+Resource::~Resource() {} // 2,3
+
+void instantiate_template() {
+  Resource x;
+}
+)cpp");
+
+  auto AST = tooling::buildASTFromCode(Code.code());
+  ASTContext  = AST->getASTContext();
+
+  const auto  = Ctx.getSourceManager();
+  auto GetNameInfoRange = [](const BoundNodes ) {
+const auto *D = Match.getNodeAs("dtor");
+return D->getNameInfo().getSourceRange().printToString(SM);
+  };
+
+  auto Matches = match(findAll(cxxDestructorDecl().bind("dtor")),
+   *Ctx.getTranslationUnitDecl(), Ctx);
+  ASSERT_EQ(Matches.size(), 3U);
+  EXPECT_EQ(GetNameInfoRange(Matches[0]), "");
+  EXPECT_EQ(GetNameInfoRange(Matches[1]), "");
+  EXPECT_EQ(GetNameInfoRange(Matches[2]), "");
+}

>From ca454ba5c74e7bfdce2a4bfbfa55b0ad7b0c814e Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Tue, 21 May 2024 09:23:40 +0200
Subject: [PATCH 2/2] NFC Add release docs for this fix

---
 clang/docs/ReleaseNotes.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2f83f5c6d54e9..7f4b1d2a00df7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -770,6 +770,7 @@ Miscellaneous Bug Fixes
 
 - Fixed an infinite recursion in ASTImporter, on return type declared inside
   body of C++11 lambda without trailing return (#GH68775).
+- Fixed declaration name source location of instantiated function definitions 
(GH71161).
 
 Miscellaneous Clang Crashes Fixed
 ^

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST] Fix end location of DeclarationNameInfo on instantiated methods (PR #92654)

[Balazs Benics via cfe-commits]

https://github.com/steakhal created 
https://github.com/llvm/llvm-project/pull/92654

Fixes #71161

[D64087](https://reviews.llvm.org/D64087) updated some locations of the 
instantiated method but forgot `DNLoc`.

`FunctionDecl::getNameInfo()` constructs a `DeclarationNameInfo` using 
`Decl::Loc` as the beginning of the declaration name, and `FunctionDecl::DNLoc` 
to compute the end of the declaration name. The former was updated, but the 
latter was not, so `DeclarationName::getSourceRange()` would return a range 
where the end of the declaration name could come before its beginning.

Patch by Alejandro Alvarez Ayllon
Co-authored-by: steakhal

CPP-5166

>From 58ca4d9be1dbc43ad40b6e26b8a5d79e20be8d93 Mon Sep 17 00:00:00 2001
From: Alejandro _lvarez Ayll_n 
Date: Sat, 18 May 2024 16:53:33 +0200
Subject: [PATCH] [clang][AST] Fix end location of DeclarationNameInfo on
 instantiated methods

Fixes #71161

[D64087](https://reviews.llvm.org/D64087) updated some locations of the
instantiated method but forgot `DNLoc`.

`FunctionDecl::getNameInfo()` constructs a `DeclarationNameInfo` using
`Decl::Loc` as the beginning of the declaration name, and
`FunctionDecl::DNLoc` to compute the end of the declaration name. The
former was updated, but the latter was not, so
`DeclarationName::getSourceRange()` would return a range where the end
of the declaration name could come before its beginning.

Patch by Alejandro Alvarez Ayllon
Co-authored-by: steakhal

CPP-5166
---
 clang/include/clang/AST/Decl.h|  2 ++
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  1 +
 clang/unittests/AST/DeclTest.cpp  | 31 +++
 3 files changed, 34 insertions(+)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 5e485ccb85a13..7fd80b90d1033 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2188,6 +2188,8 @@ class FunctionDecl : public DeclaratorDecl,
 
   void setRangeEnd(SourceLocation E) { EndRangeLoc = E; }
 
+  void setDeclarationNameLoc(DeclarationNameLoc L) { DNLoc = L; }
+
   /// Returns the location of the ellipsis of a variadic function.
   SourceLocation getEllipsisLoc() const {
 const auto *FPT = getType()->getAs();
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 381d79b2fcd46..6d736d0771eac 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5055,6 +5055,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation 
PointOfInstantiation,
   Function->setLocation(PatternDecl->getLocation());
   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
   Function->setRangeEnd(PatternDecl->getEndLoc());
+  Function->setDeclarationNameLoc(PatternDecl->getNameInfo().getInfo());
 
   EnterExpressionEvaluationContext EvalContext(
   *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp
index 2530ce74eb6a3..16aa2b50b7a06 100644
--- a/clang/unittests/AST/DeclTest.cpp
+++ b/clang/unittests/AST/DeclTest.cpp
@@ -545,3 +545,34 @@ TEST(Decl, TemplateArgumentDefaulted) {
   EXPECT_TRUE(ArgList.get(2).getIsDefaulted());
   EXPECT_TRUE(ArgList.get(3).getIsDefaulted());
 }
+
+TEST(Decl, CXXDestructorDeclsShouldHaveWellFormedNameInfoRanges) {
+  // GH71161
+  llvm::Annotations Code(R"cpp(
+template  struct Resource {
+  ~Resource(); // 1
+};
+template 
+Resource::~Resource() {} // 2,3
+
+void instantiate_template() {
+  Resource x;
+}
+)cpp");
+
+  auto AST = tooling::buildASTFromCode(Code.code());
+  ASTContext  = AST->getASTContext();
+
+  const auto  = Ctx.getSourceManager();
+  auto GetNameInfoRange = [](const BoundNodes ) {
+const auto *D = Match.getNodeAs("dtor");
+return D->getNameInfo().getSourceRange().printToString(SM);
+  };
+
+  auto Matches = match(findAll(cxxDestructorDecl().bind("dtor")),
+   *Ctx.getTranslationUnitDecl(), Ctx);
+  ASSERT_EQ(Matches.size(), 3U);
+  EXPECT_EQ(GetNameInfoRange(Matches[0]), "");
+  EXPECT_EQ(GetNameInfoRange(Matches[1]), "");
+  EXPECT_EQ(GetNameInfoRange(Matches[2]), "");
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.security.cert.pos.34c into security.PutenvWithAuto (PR #92424)

[Balazs Benics via cfe-commits]

=?utf-8?q?Bal=C3=A1zs_K=C3=A9ri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal approved this pull request.


https://github.com/llvm/llvm-project/pull/92424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.security.cert.pos.34c into security.PutenvWithAuto (PR #92424)

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/92424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.security.cert.pos.34c into security.PutenvWithAuto (PR #92424)

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -2792,6 +2792,31 @@ Warn on mmap() calls that are both writable and 
executable.
//   code
  }
 
+.. _alpha-security-putenv-stack-array:
+
+alpha.security.PutenvStackArray
+"""

steakhal wrote:

```suggestion
alpha.security.PutenvStackArray (C)
"""
```

https://github.com/llvm/llvm-project/pull/92424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.security.cert.pos.34c into security.PutenvWithAuto (PR #92424)

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -2792,6 +2792,31 @@ Warn on mmap() calls that are both writable and 
executable.
//   code
  }
 
+.. _alpha-security-putenv-stack-array:
+
+alpha.security.PutenvStackArray
+"""
+Finds calls to the ``putenv`` function which pass a pointer to a 
stack-allocated
+(automatic) array as the argument. Function ``putenv`` does not copy the passed
+string, only a pointer to the data is stored and this data can be read even by
+other threads. Content of a stack-allocated array is likely to be overwritten
+after returning from the parent function.
+
+The problem can be solved by using a static array variable or dynamically
+allocated memory. Even better is to avoid using ``putenv`` (it has other
+problems related to memory leaks) and use ``setenv`` instead.
+
+The check corresponds to CERT rule
+`POS34-C. Do not call putenv() with a pointer to an automatic variable as the 
argument
+`_.
+
+.. code-block:: c
+
+  int f() {
+char[] env = "NAME=value";

steakhal wrote:

```suggestion
char env[] = "NAME=value";
```

https://github.com/llvm/llvm-project/pull/92424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.security.cert.pos.34c into security.PutenvWithAuto (PR #92424)

[Balazs Benics via cfe-commits]

@@ -1179,6 +1179,54 @@ security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C)
strncpy(buf, "a", 1); // warn
  }
 
+.. _security-putenv-with-auto:
+
+security.PutenvWithAuto
+"""
+Finds calls to the ``putenv`` function which pass a pointer to an automatic 
variable as the argument.
+Function ``putenv`` does not copy the passed string, only a pointer to the 
data is stored.
+Content of an automatic variable is likely to be overwritten after returning 
from the parent function.

steakhal wrote:

Even though it's formally called `automatic storage duration`, I'd say that 
`stack`-variable is more commonly understood among programmers.
Consequently, I'd suggest `security.PutenvWithStack` or 
`security.PutenvWithStackVar` instead. I think it would be easier to discover 
that way.
But I guess, this should be discussed separately.

https://github.com/llvm/llvm-project/pull/92424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.security.cert.pos.34c into security.PutenvWithAuto (PR #92424)

[Balazs Benics via cfe-commits]

steakhal wrote:

> The "cert" package looks not useful and the checker has not a meaningful name 
> with the old naming scheme.
> Additionally tests and documentation is updated.
> The checker looks good enough to be moved into non-alpha package.

Personally, I prefer reviewing content changes separately from deciding/moving 
the checker out of alpha. Here are the reasons for doing so:
 - When content is moved around, a different set of mistakes can be made than 
when updating some doc content.
 - These are orthogonal decisions.

---

I'd suggest to split this PR into two:
 - Keep this PR for updating the content.
 - Have a separate PR for moving it out of alpha.



https://github.com/llvm/llvm-project/pull/92424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.security.cert.pos.34c into security.PutenvWithAuto (PR #92424)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/92424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Adding taint analysis capability to unix.Malloc checker (PR #92420)

[Balazs Benics via cfe-commits]

@@ -1273,6 +1273,41 @@ Check for memory leaks, double free, and use-after-free 
problems. Traces memory
 .. literalinclude:: checkers/unix_malloc_example.c
 :language: c
 
+If the ``alpha.security.taint.TaintPropagation`` checker is enabled, the 
checker
+warns for cases when the ``size`` parameter of the ``malloc`` , ``calloc``,
+``realloc``, ``alloca`` is tainted (potentially attacker controlled). If an
+attacker can inject a large value as the size parameter, memory exhaustion
+denial of service attack can be carried out.
+
+The analyzer emits warning only if it cannot prove that the size parameter is
+within reasonable bounds (``<= SIZE_MAX/4``). This functionality partially
+covers the SEI Cert coding standard rule `INT04-C
+`_.
+
+You can silence this warning either by bound checking the ``size`` parameter, 
or
+by explicitly marking the ``size`` parameter as sanitized. See the
+:ref:`alpha-security-taint-TaintPropagation` checker for more details.
+
+.. code-block:: c
+
+  void t1(void) {
+size_t size;
+scanf("%zu", );
+int *p = malloc(size); // warn: malloc is called with a tainted 
(potentially attacker controlled) value
+free(p);
+  }
+
+  void t3(void) {
+size_t size;
+scanf("%zu", );
+if (1024https://github.com/llvm/llvm-project/pull/92420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Adding taint analysis capability to unix.Malloc checker (PR #92420)

[Balazs Benics via cfe-commits]

@@ -1779,18 +1790,79 @@ ProgramStateRef 
MallocChecker::MallocMemAux(CheckerContext ,
 const CallEvent ,
 const Expr *SizeEx, SVal Init,
 ProgramStateRef State,
-AllocationFamily Family) {
+AllocationFamily Family) const {
   if (!State)
 return nullptr;
 
   assert(SizeEx);
   return MallocMemAux(C, Call, C.getSVal(SizeEx), Init, State, Family);
 }
 
+void MallocChecker::reportTaintBug(StringRef Msg, ProgramStateRef State,
+   CheckerContext ,
+   llvm::ArrayRef TaintedSyms,
+   AllocationFamily Family,
+   const Expr *SizeEx) const {
+  if (ExplodedNode *N = C.generateErrorNode(State)) {
+
+std::optional CheckKind =
+getCheckIfTracked(Family);
+if (!CheckKind)
+  return;
+if (!BT_TaintedAlloc[*CheckKind])
+  BT_TaintedAlloc[*CheckKind].reset(new BugType(CheckNames[*CheckKind],
+"Tainted Memory 
Allocation",
+categories::MemoryError));
+auto R = std::make_unique(
+*BT_TaintedAlloc[*CheckKind], Msg, N);
+
+bugreporter::trackExpressionValue(N, SizeEx, *R);
+for (auto Sym : TaintedSyms)
+  R->markInteresting(Sym);
+C.emitReport(std::move(R));
+  }
+}
+
+void MallocChecker::CheckTaintedness(CheckerContext , const CallEvent ,
+ const SVal SizeSVal, ProgramStateRef 
State,
+ AllocationFamily Family) const {
+  std::vector TaintedSyms =
+  clang::ento::taint::getTaintedSymbols(State, SizeSVal);
+  if (!TaintedSyms.empty()) {

steakhal wrote:

Transform this into an early-return to reduce indentation.

https://github.com/llvm/llvm-project/pull/92420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Adding taint analysis capability to unix.Malloc checker (PR #92420)

[Balazs Benics via cfe-commits]

@@ -48,6 +49,45 @@ void myfoo(int *p);
 void myfooint(int p);
 char *fooRetPtr(void);
 
+void t1(void) {
+  size_t size;
+  scanf("%zu", );
+  int *p = malloc(size); // expected-warning{{malloc is called with a tainted 
(potentially attacker controlled) value}}
+  free(p);
+}
+
+void t2(void) {
+  size_t size;
+  scanf("%zu", );
+  int *p = calloc(size,2); // expected-warning{{calloc is called with a 
tainted (potentially attacker controlled) value}}
+  free(p);
+}
+
+void t3(void) {
+  size_t size;
+  scanf("%zu", );
+  if (1024https://github.com/llvm/llvm-project/pull/92420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Adding taint analysis capability to unix.Malloc checker (PR #92420)

[Balazs Benics via cfe-commits]

@@ -1779,18 +1790,79 @@ ProgramStateRef 
MallocChecker::MallocMemAux(CheckerContext ,
 const CallEvent ,
 const Expr *SizeEx, SVal Init,
 ProgramStateRef State,
-AllocationFamily Family) {
+AllocationFamily Family) const {
   if (!State)
 return nullptr;
 
   assert(SizeEx);
   return MallocMemAux(C, Call, C.getSVal(SizeEx), Init, State, Family);
 }
 
+void MallocChecker::reportTaintBug(StringRef Msg, ProgramStateRef State,
+   CheckerContext ,
+   llvm::ArrayRef TaintedSyms,
+   AllocationFamily Family,
+   const Expr *SizeEx) const {
+  if (ExplodedNode *N = C.generateErrorNode(State)) {
+
+std::optional CheckKind =
+getCheckIfTracked(Family);
+if (!CheckKind)
+  return;
+if (!BT_TaintedAlloc[*CheckKind])
+  BT_TaintedAlloc[*CheckKind].reset(new BugType(CheckNames[*CheckKind],
+"Tainted Memory 
Allocation",
+categories::MemoryError));
+auto R = std::make_unique(
+*BT_TaintedAlloc[*CheckKind], Msg, N);
+
+bugreporter::trackExpressionValue(N, SizeEx, *R);
+for (auto Sym : TaintedSyms)
+  R->markInteresting(Sym);
+C.emitReport(std::move(R));
+  }
+}
+
+void MallocChecker::CheckTaintedness(CheckerContext , const CallEvent ,
+ const SVal SizeSVal, ProgramStateRef 
State,
+ AllocationFamily Family) const {
+  std::vector TaintedSyms =
+  clang::ento::taint::getTaintedSymbols(State, SizeSVal);

steakhal wrote:

```suggestion
  taint::getTaintedSymbols(State, SizeSVal);
```

https://github.com/llvm/llvm-project/pull/92420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Adding taint analysis capability to unix.Malloc checker (PR #92420)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/92420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Adding taint analysis capability to unix.Malloc checker (PR #92420)

[Balazs Benics via cfe-commits]

@@ -1273,6 +1273,41 @@ Check for memory leaks, double free, and use-after-free 
problems. Traces memory
 .. literalinclude:: checkers/unix_malloc_example.c
 :language: c
 
+If the ``alpha.security.taint.TaintPropagation`` checker is enabled, the 
checker
+warns for cases when the ``size`` parameter of the ``malloc`` , ``calloc``,
+``realloc``, ``alloca`` is tainted (potentially attacker controlled). If an
+attacker can inject a large value as the size parameter, memory exhaustion
+denial of service attack can be carried out.
+
+The analyzer emits warning only if it cannot prove that the size parameter is
+within reasonable bounds (``<= SIZE_MAX/4``). This functionality partially
+covers the SEI Cert coding standard rule `INT04-C
+`_.
+
+You can silence this warning either by bound checking the ``size`` parameter, 
or
+by explicitly marking the ``size`` parameter as sanitized. See the
+:ref:`alpha-security-taint-TaintPropagation` checker for more details.
+
+.. code-block:: c
+
+  void t1(void) {
+size_t size;
+scanf("%zu", );
+int *p = malloc(size); // warn: malloc is called with a tainted 
(potentially attacker controlled) value
+free(p);
+  }
+
+  void t3(void) {
+size_t size;
+scanf("%zu", );
+if (1024https://github.com/llvm/llvm-project/pull/92420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Adding taint analysis capability to unix.Malloc checker (PR #92420)

[Balazs Benics via cfe-commits]

@@ -48,6 +49,45 @@ void myfoo(int *p);
 void myfooint(int p);
 char *fooRetPtr(void);
 
+void t1(void) {
+  size_t size;
+  scanf("%zu", );
+  int *p = malloc(size); // expected-warning{{malloc is called with a tainted 
(potentially attacker controlled) value}}
+  free(p);
+}
+
+void t2(void) {
+  size_t size;
+  scanf("%zu", );
+  int *p = calloc(size,2); // expected-warning{{calloc is called with a 
tainted (potentially attacker controlled) value}}
+  free(p);
+}
+
+void t3(void) {
+  size_t size;
+  scanf("%zu", );
+  if (1024https://github.com/llvm/llvm-project/pull/92420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Adding taint analysis capability to unix.Malloc checker (PR #92420)

[Balazs Benics via cfe-commits]

https://github.com/steakhal commented:

The patch makes sense to me.
I'll not repeat the existing comments, they raise relevant concerns.

It would be nice to extend some test case with a tainted malloc to see how 
those note tags play out from the generic taint checker in this context. For 
this, I'd suggest you to have a look at some taint tests where we enable the 
`text` diagnostic output.

https://github.com/llvm/llvm-project/pull/92420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Require explicit matching mode for CallDescriptions (PR #92454)

[Balazs Benics via cfe-commits]

=?utf-8?q?Don=C3=A1t?= Nagy 
Message-ID:
In-Reply-To: 


https://github.com/steakhal approved this pull request.


https://github.com/llvm/llvm-project/pull/92454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,75 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,security.SetgidSetuidOrder 
-analyzer-output=text -verify %s
+
+typedef int uid_t;
+typedef int gid_t;
+
+int setuid(uid_t);
+int setgid(gid_t);
+
+uid_t getuid();
+gid_t getgid();
+
+
+
+void test_note_1() {
+  if (setuid(getuid()) == -1) // expected-note{{Assuming the condition is 
false}} \
+  // expected-note{{Taking false branch}}
+return;
+  if (setuid(getuid()) == -1) // expected-note{{Call to 'setuid' found here 
that removes superuser privileges}} \
+  // expected-note{{Assuming the condition is 
false}} \
+  // expected-note{{Taking false branch}}
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}} \
+  // expected-note{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+}
+
+void test_note_2() {
+  if (setuid(getuid()) == -1) // expected-note{{Call to 'setuid' found here 
that removes superuser privileges}} \
+  // expected-note{{Assuming the condition is 
false}} \
+  // expected-note{{Taking false branch}} \
+  // expected-note{{Assuming the condition is 
false}} \
+  // expected-note{{Taking false branch}}

steakhal wrote:

```suggestion
  // expected-note 2 {{Assuming the condition is 
false}} \
  // expected-note 2 {{Taking false branch}}
```

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal approved this pull request.

LGTM now, modulo the license concern mentioned 
[here](https://github.com/llvm/llvm-project/pull/91445#discussion_r1599766388).
I approve this, given that is checked/resolved.

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [clang] Fix CXXNewExpr end source location for 'new struct S' (PR #92266)

[Balazs Benics via cfe-commits]

https://github.com/steakhal closed 
https://github.com/llvm/llvm-project/pull/92266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker : public Checker 
{
+  const BugType BT{this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  const CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+  const BugType *getBT() const { return  }
+
+private:
+  void processSetuid(ProgramStateRef State, const CallEvent ,
+ CheckerContext ) const;
+  void processSetgid(ProgramStateRef State, const CallEvent ,
+ CheckerContext ) const;
+  void processOther(ProgramStateRef State, const CallEvent ,
+CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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), and for identification of note
+/// tags.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(LastSetuidCallSVal, SymbolRef)
+
+void SetgidSetuidOrderChecker::checkPostCall(const CallEvent ,
+ CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+  if (SetuidDesc.matches(Call)) {
+processSetuid(State, Call, C);
+  } else if (SetgidDesc.matches(Call)) {
+processSetgid(State, Call, C);
+  } else if (OtherSetPrivilegeDesc.contains(Call)) {
+processOther(State, Call, C);
+  }
+}
+
+ProgramStateRef SetgidSetuidOrderChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+ bool Assumption) const {
+  SValBuilder  = State->getStateManager().getSValBuilder();
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return State;
+
+  // Check if the most recent call to 'setuid(getuid())' is assumed to be != 0.
+  // It should be only -1 at failure, but we want to accept a "!= 0" check too.
+  // (But now an invalid failure check like "!= 1" will be recognized as 
correct
+  // too. The "invalid failure check" is a different bug that is not the 

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker : public Checker 
{
+  const BugType BT{this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  const CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+  const BugType *getBT() const { return  }

steakhal wrote:

I guess you dont need this.

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal requested changes to this pull request.

NoteTags, yeey.

Please add tests for the Notes added by the NoteTag callbacks. Inother words, 
add a test with the text diagnostic output.

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix CXXNewExpr end source location for 'new struct S' (PR #92266)

[Balazs Benics via cfe-commits]

https://github.com/steakhal updated 
https://github.com/llvm/llvm-project/pull/92266

>From eeb24ddbf261fd7667dd05feee14637bc379d182 Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh 
Date: Wed, 15 May 2024 16:02:07 +0200
Subject: [PATCH] Fix CXXNewExpr end source location for 'new struct S'

---
 clang/lib/Parse/ParseDeclCXX.cpp  | 1 +
 clang/test/Misc/ast-source-ranges.cpp | 5 +
 2 files changed, 6 insertions(+)
 create mode 100644 clang/test/Misc/ast-source-ranges.cpp

diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 65ddebca49bc6..32c4e923243a9 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -1883,6 +1883,7 @@ void Parser::ParseClassSpecifier(tok::TokenKind 
TagTokKind,
   if (Tok.is(tok::identifier)) {
 Name = Tok.getIdentifierInfo();
 NameLoc = ConsumeToken();
+DS.SetRangeEnd(NameLoc);
 
 if (Tok.is(tok::less) && getLangOpts().CPlusPlus) {
   // The name was supposed to refer to a template, but didn't.
diff --git a/clang/test/Misc/ast-source-ranges.cpp 
b/clang/test/Misc/ast-source-ranges.cpp
new file mode 100644
index 0..9c0ab9449a6f5
--- /dev/null
+++ b/clang/test/Misc/ast-source-ranges.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -ast-dump %s  2>&1 | FileCheck %s
+
+struct Sock {};
+void leakNewFn() { new struct Sock; }
+// CHECK: CXXNewExpr {{.*}}  'struct Sock *'

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix CXXNewExpr end source location for 'new struct S' (PR #92266)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/92266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Fix a crash in alpha.unix.BlockInCriticalSection (PR #90030)

[Balazs Benics via cfe-commits]

Endre =?utf-8?q?Fülöp?= ,Balazs
 Benics 
Message-ID:
In-Reply-To: 


https://github.com/steakhal closed 
https://github.com/llvm/llvm-project/pull/90030
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Fix a crash in alpha.unix.BlockInCriticalSection (PR #90030)

[Balazs Benics via cfe-commits]

Endre =?utf-8?q?Fülöp?= ,Balazs
 Benics 
Message-ID:
In-Reply-To: 


https://github.com/steakhal updated 
https://github.com/llvm/llvm-project/pull/90030

>From af05be993f4789705cde374dbf7efefd9a18f1c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= 
Date: Tue, 9 Apr 2024 10:44:43 +0200
Subject: [PATCH 1/3] [clang][analyzer] Fix alpha.unix.BlockInCriticalSection

When analyzing C code with function pointers the checker crashes because
of how the implementation extracts IdentifierInfo. Without the fix, this
test crashes.

Add crashing test
---
 .../Checkers/BlockInCriticalSectionChecker.cpp| 8 +---
 clang/test/Analysis/block-in-critical-section.c   | 6 ++
 2 files changed, 11 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Analysis/block-in-critical-section.c

diff --git 
a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index e138debd1361c..d381a30f7e24c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -14,6 +14,7 @@
 //
 
//===--===//
 
+#include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -103,9 +104,10 @@ class RAIIMutexDescriptor {
   // this function is called instead of early returning it. To avoid this, 
a
   // bool variable (IdentifierInfoInitialized) is used and the function 
will
   // be run only once.
-  Guard = ()->getASTContext().Idents.get(
-  GuardName);
-  IdentifierInfoInitialized = true;
+  if (AnalysisDeclContext *CalleCtx = Call.getCalleeAnalysisDeclContext()) 
{
+Guard = >getASTContext().Idents.get(GuardName);
+IdentifierInfoInitialized = true;
+  }
 }
   }
 
diff --git a/clang/test/Analysis/block-in-critical-section.c 
b/clang/test/Analysis/block-in-critical-section.c
new file mode 100644
index 0..1e174af541b18
--- /dev/null
+++ b/clang/test/Analysis/block-in-critical-section.c
@@ -0,0 +1,6 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.unix.BlockInCriticalSection -verify %s
+// expected-no-diagnostics
+
+// This should not crash
+int (*a)(void);
+void b(void) { a(); }

>From a18c0900f438730c3bf25ac44ceac156fd416a12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= 
Date: Wed, 15 May 2024 12:08:33 +0200
Subject: [PATCH 2/3] Get ASTContext through StateManager

---
 .../Checkers/BlockInCriticalSectionChecker.cpp  | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git 
a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index d381a30f7e24c..c57ca262d2484 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -104,10 +104,8 @@ class RAIIMutexDescriptor {
   // this function is called instead of early returning it. To avoid this, 
a
   // bool variable (IdentifierInfoInitialized) is used and the function 
will
   // be run only once.
-  if (AnalysisDeclContext *CalleCtx = Call.getCalleeAnalysisDeclContext()) 
{
-Guard = >getASTContext().Idents.get(GuardName);
-IdentifierInfoInitialized = true;
-  }
+  const auto  = Call.getState()->getStateManager().getContext();
+  Guard = (GuardName);
 }
   }
 

>From c6c96953c22366edd7cc6e9cb0afea7c5374d19f Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Wed, 15 May 2024 15:41:25 +0200
Subject: [PATCH 3/3] Remove unused include

---
 .../StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp| 1 -
 1 file changed, 1 deletion(-)

diff --git 
a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index c57ca262d2484..92347f8fafc00 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -14,7 +14,6 @@
 //
 
//===--===//
 
-#include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Fix a crash in alpha.unix.BlockInCriticalSection (PR #90030)

[Balazs Benics via cfe-commits]

Endre =?utf-8?q?Fülöp?= 
Message-ID:
In-Reply-To: 



@@ -14,6 +14,7 @@
 //
 
//===--===//
 
+#include "clang/Analysis/AnalysisDeclContext.h"

steakhal wrote:

```suggestion
```
You can probably drop this line now.

https://github.com/llvm/llvm-project/pull/90030
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Fix a crash in alpha.unix.BlockInCriticalSection (PR #90030)

[Balazs Benics via cfe-commits]

Endre =?utf-8?q?F=C3=BCl=C3=B6p?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal approved this pull request.

LGTM, thanks!

https://github.com/llvm/llvm-project/pull/90030
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Fix a crash in alpha.unix.BlockInCriticalSection (PR #90030)

[Balazs Benics via cfe-commits]

Endre =?utf-8?q?Fülöp?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/90030
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Fix a crash in alpha.unix.BlockInCriticalSection (PR #90030)

[Balazs Benics via cfe-commits]

@@ -103,9 +104,10 @@ class RAIIMutexDescriptor {
   // this function is called instead of early returning it. To avoid this, 
a
   // bool variable (IdentifierInfoInitialized) is used and the function 
will
   // be run only once.
-  Guard = ()->getASTContext().Idents.get(
-  GuardName);
-  IdentifierInfoInitialized = true;
+  if (AnalysisDeclContext *CalleCtx = Call.getCalleeAnalysisDeclContext()) 
{
+Guard = >getASTContext().Idents.get(GuardName);
+IdentifierInfoInitialized = true;
+  }

steakhal wrote:

This crash happened because we failed to get an ASTContext.
Maybe, on some other access-path we could acquire the ASTContext, like via 
`Call.getState()->getStateManager().getContext()`.

I feel like your proposal only fixes the symptom, and not the root cause.

https://github.com/llvm/llvm-project/pull/90030
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Fix a crash in alpha.unix.BlockInCriticalSection (PR #90030)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/90030
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Fix a crash in alpha.unix.BlockInCriticalSection (PR #90030)

[Balazs Benics via cfe-commits]

https://github.com/steakhal requested changes to this pull request.

Now the change makes sense. I disagree with the fix though, see inline.

https://github.com/llvm/llvm-project/pull/90030
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Fix a crash in alpha.unix.BlockInCriticalSection (PR #90030)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/90030
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

[Balazs Benics via cfe-commits]

=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy 
Message-ID:
In-Reply-To: 


https://github.com/steakhal approved this pull request.


https://github.com/llvm/llvm-project/pull/91531
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

[Balazs Benics via cfe-commits]

@@ -572,196 +570,236 @@ void GenericTaintChecker::initTaintRules(CheckerContext 
) const {
   std::vector>;
   using TR = GenericTaintRule;
 
-  const Builtin::Context  = C.getASTContext().BuiltinInfo;
-
   RulesConstructionTy GlobalCRules{
   // Sources
-  {{{"fdopen"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"fopen"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"freopen"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getch"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getchar"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getchar_unlocked"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"gets"}}, TR::Source({{0}, ReturnValueIndex})},
-  {{{"gets_s"}}, TR::Source({{0}, ReturnValueIndex})},
-  {{{"scanf"}}, TR::Source({{}, 1})},
-  {{{"scanf_s"}}, TR::Source({{}, {1}})},
-  {{{"wgetch"}}, TR::Source({{}, ReturnValueIndex})},
+  {{CDM::CLibrary, {"fdopen"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"fopen"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"freopen"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getch"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getchar"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getchar_unlocked"}}, 
TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"gets"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"gets_s"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"scanf"}}, TR::Source({{}, 1})},
+  {{CDM::CLibrary, {"scanf_s"}}, TR::Source({{}, 1})},
+  {{CDM::CLibrary, {"wgetch"}}, TR::Source({{ReturnValueIndex}})},
   // Sometimes the line between taint sources and propagators is blurry.
   // _IO_getc is choosen to be a source, but could also be a propagator.
   // This way it is simpler, as modeling it as a propagator would require
   // to model the possible sources of _IO_FILE * values, which the _IO_getc
   // function takes as parameters.
-  {{{"_IO_getc"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getcwd"}}, TR::Source({{0, ReturnValueIndex}})},
-  {{{"getwd"}}, TR::Source({{0, ReturnValueIndex}})},
-  {{{"readlink"}}, TR::Source({{1, ReturnValueIndex}})},
-  {{{"readlinkat"}}, TR::Source({{2, ReturnValueIndex}})},
-  {{{"get_current_dir_name"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"gethostname"}}, TR::Source({{0}})},
-  {{{"getnameinfo"}}, TR::Source({{2, 4}})},
-  {{{"getseuserbyname"}}, TR::Source({{1, 2}})},
-  {{{"getgroups"}}, TR::Source({{1, ReturnValueIndex}})},
-  {{{"getlogin"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getlogin_r"}}, TR::Source({{0}})},
+  {{CDM::CLibrary, {"_IO_getc"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getcwd"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getwd"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"readlink"}}, TR::Source({{1, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"readlinkat"}}, TR::Source({{2, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"get_current_dir_name"}},
+   TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"gethostname"}}, TR::Source({{0}})},
+  {{CDM::CLibrary, {"getnameinfo"}}, TR::Source({{2, 4}})},
+  {{CDM::CLibrary, {"getseuserbyname"}}, TR::Source({{1, 2}})},
+  {{CDM::CLibrary, {"getgroups"}}, TR::Source({{1, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getlogin"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getlogin_r"}}, TR::Source({{0}})},
 
   // Props
-  {{{"accept"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"atoi"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"atol"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"atoll"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fgetc"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fgetln"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fgets"}}, TR::Prop({{2}}, {{0, ReturnValueIndex}})},
-  {{{"fgetws"}}, TR::Prop({{2}}, {{0, ReturnValueIndex}})},
-  {{{"fscanf"}}, TR::Prop({{0}}, {{}, 2})},
-  {{{"fscanf_s"}}, TR::Prop({{0}}, {{}, {2}})},
-  {{{"sscanf"}}, TR::Prop({{0}}, {{}, 2})},
-
-  {{{"getc"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"getc_unlocked"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"getdelim"}}, TR::Prop({{3}}, {{0}})},
-  {{{"getline"}}, TR::Prop({{2}}, {{0}})},
-  {{{"getw"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"pread"}}, TR::Prop({{0, 1, 2, 3}}, {{1, ReturnValueIndex}})},
-  {{{"read"}}, TR::Prop({{0, 2}}, {{1, ReturnValueIndex}})},
-  {{{"strchr"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"strrchr"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"tolower"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"toupper"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fread"}}, TR::Prop({{3}}, {{0, 

[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

[Balazs Benics via cfe-commits]

@@ -400,17 +400,14 @@ class GenericTaintChecker : public 
Checker {
   void taintUnsafeSocketProtocol(const CallEvent ,
  CheckerContext ) const;
 
-  /// Default taint rules are initalized with the help of a CheckerContext to
-  /// access the names of built-in functions like memcpy.
+  /// The taint rules are initalized with the help of a CheckerContext to
+  /// access user-provided configuration.
   void initTaintRules(CheckerContext ) const;
 
-  /// CallDescription currently cannot restrict matches to the global namespace
-  /// only, which is why multiple CallDescriptionMaps are used, as we want to
-  /// disambiguate global C functions from functions inside user-defined
-  /// namespaces.
-  // TODO: Remove separation to simplify matching logic once CallDescriptions
-  // are more expressive.
-
+  // TODO: The two separate `CallDescriptionMap`s were introduced when
+  // `CallDescription` was unable to restric matches to the global namespace

steakhal wrote:

```suggestion
  // `CallDescription` was unable to restrict matches to the global namespace
```

https://github.com/llvm/llvm-project/pull/91635
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

[Balazs Benics via cfe-commits]

https://github.com/steakhal approved this pull request.

LGTM.
I haven't checked absolutely everything, rather sampled.
It looked correct, and free of copy-paste mistakes.

https://github.com/llvm/llvm-project/pull/91635
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91635
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent ,
+   CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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 ,
+ CheckerContext ) 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 (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});
+  C.addTransition(State, C.getPredecessor());
+}
+
+ProgramStateRef SetgidSetuidOrderChecker::evalAssume(ProgramStateRef State,
+ SVal 

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -1179,6 +1179,34 @@ security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C)
strncpy(buf, "a", 1); // warn
  }
 
+security.SetgidSetuidOrder (C)
+""
+When dropping user-level and group-level privileges in a program by using
+``setuid`` and ``setgid`` calls, it is important to reset the group-level
+privileges (with ``setgid``) first. Function ``setgid`` will likely fail if
+the superuser privileges are already dropped.
+
+The checker checks for sequences of ``setuid(getuid())`` and
+``setgid(getgid())`` calls (in this order). If such a sequence is found and
+there is no other privilege-changing function call (``seteuid``, ``setreuid``,
+``setresuid`` and the GID versions of these) in between, a warning is
+generated. The checker finds only exactly ``setuid(getuid())`` calls (and the
+GID versions), not for example if the result of ``getuid()`` is stored in a
+variable.
+
+This check corresponds to SEI CERT Rule `POS36-C 
`_.
+
+.. code-block:: c
+
+ void test1() {
+   if (setuid(getuid()) == -1) {
+ /* handle error condition */
+   }
+   if (setgid(getgid()) == -1) { // warn
+ /* handle error condition */
+   }
+ }
+

steakhal wrote:

I think the docs should explicitly mention how to fix/suppress this issue.
Like: One should call `setgid(getgid())` first and then `setuid(getuid())`.
Maybe also mention to pay attention to not mix up the APIs so that user ID and 
group IDs are not swapped, etc. Also add a remark to not forget to check the 
return value of these APIs. 

And this example looks suspiciously similar to the one present in the CERT docs.
Does it raise license and copy-right issues? If that's a concern, please rework 
the example.

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal approved this pull request.

I think this looks good now.
I think to really reach the full potential of this checker, we must have a 
visitor/note tag for highlighting the places of the last `setgid` or `setuid` 
call in the report.

I also think that the docs could be improved.

Anyways. I think it's already pretty good, and I don't plan to do another round 
of reviews.

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -1011,6 +1011,11 @@ def FloatLoopCounter : Checker<"FloatLoopCounter">,
   Dependencies<[SecuritySyntaxChecker]>,
   Documentation;
 
+def SetgidSetuidOrderChecker : Checker<"SetgidSetuidOrder">,
+  HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and 
'setuid(getuid())' (CERT: "
+   "POS36-C)">,

steakhal wrote:

```suggestion
  HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and "
   "'setuid(getuid())' (CERT: POS36-C)">,
```
The previous line-brake seemed so arbitrary. This way we at least obey the 80 
column rule.

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -30,23 +30,20 @@ enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid 
};
 
 class SetgidSetuidOrderChecker
 : public Checker {
-  const BugType BT_WrongRevocationOrder{
-  this, "Possible wrong order of privilege revocation"};
+  const BugType BT{this, "Possible wrong order of privilege revocation"};
 
   const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
   const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
 
   const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
   const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
 
-  CallDescriptionSet OtherSetPrivilegeDesc{
+  CallDescriptionSet const OtherSetPrivilegeDesc{

steakhal wrote:

I think in LLVM we usually use west-const.
```suggestion
  const CallDescriptionSet OtherSetPrivilegeDesc{
```

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

[Balazs Benics via cfe-commits]

=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 



@@ -136,53 +100,49 @@ void ErrnoModeling::checkBeginFunction(CheckerContext ) 
const {
   ASTContext  = C.getASTContext();
   ProgramStateRef State = C.getState();
 
-  if (const auto *ErrnoVar = dyn_cast_or_null(ErrnoDecl)) {
-// There is an external 'errno' variable.
-// Use its memory region.
-// The memory region for an 'errno'-like variable is allocated in system
-// space by MemRegionManager.
-const MemRegion *ErrnoR =
-State->getRegion(ErrnoVar, C.getLocationContext());
+  const MemRegion *ErrnoR;

steakhal wrote:

```suggestion
  const MemRegion *ErrnoR = nullptr;
```

https://github.com/llvm/llvm-project/pull/91531
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

[Balazs Benics via cfe-commits]

=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 



@@ -136,53 +100,49 @@ void ErrnoModeling::checkBeginFunction(CheckerContext ) 
const {
   ASTContext  = C.getASTContext();
   ProgramStateRef State = C.getState();
 
-  if (const auto *ErrnoVar = dyn_cast_or_null(ErrnoDecl)) {
-// There is an external 'errno' variable.
-// Use its memory region.
-// The memory region for an 'errno'-like variable is allocated in system
-// space by MemRegionManager.
-const MemRegion *ErrnoR =
-State->getRegion(ErrnoVar, C.getLocationContext());
+  const MemRegion *ErrnoR;
+
+  if (ErrnoDecl) {
+// There is an external 'errno' variable, so we can simply use the memory
+// region that's associated with it.
+ErrnoR = State->getRegion(ErrnoDecl, C.getLocationContext());
 assert(ErrnoR && "Memory region should exist for the 'errno' variable.");
-State = State->set(ErrnoR);
-State =
-errno_modeling::setErrnoValue(State, C, 0, errno_modeling::Irrelevant);
-C.addTransition(State);
-  } else if (ErrnoDecl) {
-assert(isa(ErrnoDecl) && "Invalid errno location function.");
-// There is a function that returns the location of 'errno'.
-// We must create a memory region for it in system space.
-// Currently a symbolic region is used with an artifical symbol.
-// FIXME: It is better to have a custom (new) kind of MemRegion for such
-// cases.
+  } else {
+// There is no 'errno' variable, so create a new symbolic memory region
+// that can be used to model the return value of the "get the location of
+// errno" internal functions.
+// NOTE: this `SVal` is created even if errno is not defined or used.
 SValBuilder  = C.getSValBuilder();
 MemRegionManager  = C.getStateManager().getRegionManager();
 
 const MemSpaceRegion *GlobalSystemSpace =
 RMgr.getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
 
 // Create an artifical symbol for the region.
-// It is not possible to associate a statement or expression in this case.
+// Note that it is not possible to associate a statement or expression in
+// this case and the `symbolTag` (opaque pointer tag) is just the address
+// of the data member `ErrnoDecl` of the singleton `ErrnoModeling` checker
+// object.
 const SymbolConjured *Sym = SVB.conjureSymbol(
 nullptr, C.getLocationContext(),
 ACtx.getLValueReferenceType(ACtx.IntTy), C.blockCount(), );
 
 // The symbolic region is untyped, create a typed sub-region in it.
 // The ElementRegion is used to make the errno region a typed region.
-const MemRegion *ErrnoR = RMgr.getElementRegion(
+ErrnoR = RMgr.getElementRegion(
 ACtx.IntTy, SVB.makeZeroArrayIndex(),
 RMgr.getSymbolicRegion(Sym, GlobalSystemSpace), C.getASTContext());
-State = State->set(ErrnoR);
-State =
-errno_modeling::setErrnoValue(State, C, 0, errno_modeling::Irrelevant);
-C.addTransition(State);
   }
+  State = State->set(ErrnoR);

steakhal wrote:

```suggestion
  assert(ErrnoR);
  State = State->set(ErrnoR);
```

https://github.com/llvm/llvm-project/pull/91531
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

[Balazs Benics via cfe-commits]

=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91531
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

[Balazs Benics via cfe-commits]

=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy 
Message-ID:
In-Reply-To: 


https://github.com/steakhal approved this pull request.

LGTM.
I'd only add some safety barriers for some places, denoted my suggested edits.

https://github.com/llvm/llvm-project/pull/91531
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Ignore try-statements in UnreachableCode checker (PR #91675)

[Balazs Benics via cfe-commits]

https://github.com/steakhal closed 
https://github.com/llvm/llvm-project/pull/91675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

[Balazs Benics via cfe-commits]

@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
 case Stmt::CXXDefaultArgExprClass:
 case Stmt::CXXDefaultInitExprClass: {
   Bldr.takeNodes(Pred);
-  ExplodedNodeSet PreVisit;
-  getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+  ExplodedNodeSet CheckedSet;
+  getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, S, *this);
 
   ExplodedNodeSet Tmp;
-  StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
+  StmtNodeBuilder Bldr2(CheckedSet, Tmp, *currBldrCtx);
 
-  const Expr *ArgE;
-  if (const auto *DefE = dyn_cast(S))
+  bool HasRewrittenInit = false;
+  const Expr *ArgE = nullptr;
+  if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else if (const auto *DefE = dyn_cast(S))
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else
 llvm_unreachable("unknown constant wrapper kind");
 
-  bool IsTemporary = false;
-  if (const auto *MTE = dyn_cast(ArgE)) {
-ArgE = MTE->getSubExpr();
-IsTemporary = true;
-  }
+  if (HasRewrittenInit) {
+for (auto *I : CheckedSet) {
+  ProgramStateRef state = (*I).getState();
+  const LocationContext *LCtx = (*I).getLocationContext();
+  SVal Val = state->getSVal(ArgE, LCtx);
+  state = state->BindExpr(S, LCtx, Val);
+  Bldr2.generateNode(S, I, state);

steakhal wrote:

```suggestion
for (ExplodedNode *N : CheckedSet) {
  ProgramStateRef State = N->getState();
  const LocationContext *LCtx = N->getLocationContext();
  State = State->BindExpr(S, LCtx, State->getSVal(ArgE, LCtx));
  Bldr2.generateNode(S, N, State);
```

https://github.com/llvm/llvm-project/pull/91879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

[Balazs Benics via cfe-commits]

@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++14 -verify  %s \
+// RUN: %clang_analyze_cc1 -std=c++14 -verify=expected,pedantic  %s \

steakhal wrote:

You introduced the `pedantic` verify prefix, but never actually used it.

https://github.com/llvm/llvm-project/pull/91879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

[Balazs Benics via cfe-commits]

@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
 case Stmt::CXXDefaultArgExprClass:
 case Stmt::CXXDefaultInitExprClass: {
   Bldr.takeNodes(Pred);
-  ExplodedNodeSet PreVisit;
-  getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+  ExplodedNodeSet CheckedSet;

steakhal wrote:

I think I'd prefer the old name here. That is how one would find these in the 
other places.

https://github.com/llvm/llvm-project/pull/91879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

[Balazs Benics via cfe-commits]

@@ -1114,17 +1114,16 @@ void fCXX11MemberInitTest1() {
   CXX11MemberInitTest1();
 }
 
+#ifdef PEDANTIC

steakhal wrote:

I don't think you need to guard this section of code if you were using `// 
pedantic-note {{...}}` and `// pedantic-warning {{...}}` in the guarded checks.

I also think that the `// TODO: we'd expect the warning: {{2 uninitializeds 
field}}` comment refers to this new appearing report, so you should just drop 
that comment from the `fCXX11MemberInitTest2` function.

https://github.com/llvm/llvm-project/pull/91879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

[Balazs Benics via cfe-commits]

https://github.com/steakhal commented:

In general, this PR looks good to me.
I only have some nits inline.

If it didn't break anything, it should be good to go.

https://github.com/llvm/llvm-project/pull/91879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

[Balazs Benics via cfe-commits]

@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
 case Stmt::CXXDefaultArgExprClass:
 case Stmt::CXXDefaultInitExprClass: {
   Bldr.takeNodes(Pred);
-  ExplodedNodeSet PreVisit;
-  getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+  ExplodedNodeSet CheckedSet;
+  getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, S, *this);
 
   ExplodedNodeSet Tmp;
-  StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
+  StmtNodeBuilder Bldr2(CheckedSet, Tmp, *currBldrCtx);
 
-  const Expr *ArgE;
-  if (const auto *DefE = dyn_cast(S))
+  bool HasRewrittenInit = false;
+  const Expr *ArgE = nullptr;
+  if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else if (const auto *DefE = dyn_cast(S))
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else
 llvm_unreachable("unknown constant wrapper kind");
 
-  bool IsTemporary = false;
-  if (const auto *MTE = dyn_cast(ArgE)) {
-ArgE = MTE->getSubExpr();
-IsTemporary = true;
-  }
+  if (HasRewrittenInit) {
+for (auto *I : CheckedSet) {
+  ProgramStateRef state = (*I).getState();
+  const LocationContext *LCtx = (*I).getLocationContext();
+  SVal Val = state->getSVal(ArgE, LCtx);
+  state = state->BindExpr(S, LCtx, Val);
+  Bldr2.generateNode(S, I, state);
+}
+  } else {
+// If it's not rewritten, the contents of these expressions are not
+// actually part of the current function, so we fall back to constant
+// evaluation.
+bool IsTemporary = false;
+if (const auto *MTE = dyn_cast(ArgE)) {
+  ArgE = MTE->getSubExpr();
+  IsTemporary = true;
+}
+
+std::optional ConstantVal = svalBuilder.getConstantVal(ArgE);
+if (!ConstantVal)
+  ConstantVal = UnknownVal();

steakhal wrote:

```suggestion
```
I'd just drop this and use `value_or()` at the only use-site instead.

https://github.com/llvm/llvm-project/pull/91879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

[Balazs Benics via cfe-commits]

@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
 case Stmt::CXXDefaultArgExprClass:
 case Stmt::CXXDefaultInitExprClass: {
   Bldr.takeNodes(Pred);
-  ExplodedNodeSet PreVisit;
-  getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+  ExplodedNodeSet CheckedSet;
+  getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, S, *this);
 
   ExplodedNodeSet Tmp;
-  StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
+  StmtNodeBuilder Bldr2(CheckedSet, Tmp, *currBldrCtx);
 
-  const Expr *ArgE;
-  if (const auto *DefE = dyn_cast(S))
+  bool HasRewrittenInit = false;
+  const Expr *ArgE = nullptr;
+  if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else if (const auto *DefE = dyn_cast(S))
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else
 llvm_unreachable("unknown constant wrapper kind");
 
-  bool IsTemporary = false;
-  if (const auto *MTE = dyn_cast(ArgE)) {
-ArgE = MTE->getSubExpr();
-IsTemporary = true;
-  }
+  if (HasRewrittenInit) {
+for (auto *I : CheckedSet) {
+  ProgramStateRef state = (*I).getState();
+  const LocationContext *LCtx = (*I).getLocationContext();
+  SVal Val = state->getSVal(ArgE, LCtx);
+  state = state->BindExpr(S, LCtx, Val);
+  Bldr2.generateNode(S, I, state);
+}
+  } else {
+// If it's not rewritten, the contents of these expressions are not
+// actually part of the current function, so we fall back to constant
+// evaluation.
+bool IsTemporary = false;
+if (const auto *MTE = dyn_cast(ArgE)) {
+  ArgE = MTE->getSubExpr();
+  IsTemporary = true;
+}
+
+std::optional ConstantVal = svalBuilder.getConstantVal(ArgE);
+if (!ConstantVal)
+  ConstantVal = UnknownVal();
 
-  std::optional ConstantVal = svalBuilder.getConstantVal(ArgE);
-  if (!ConstantVal)
-ConstantVal = UnknownVal();
-
-  const LocationContext *LCtx = Pred->getLocationContext();
-  for (const auto I : PreVisit) {
-ProgramStateRef State = I->getState();
-State = State->BindExpr(S, LCtx, *ConstantVal);
-if (IsTemporary)
-  State = createTemporaryRegionIfNeeded(State, LCtx,
-cast(S),
-cast(S));
-Bldr2.generateNode(S, I, State);
+const LocationContext *LCtx = Pred->getLocationContext();
+for (auto *I : CheckedSet) {
+  ProgramStateRef State = I->getState();
+  State = State->BindExpr(S, LCtx, *ConstantVal);

steakhal wrote:

```suggestion
  State = State->BindExpr(S, LCtx, ConstantVal.value_or(UnknownVal()));
```

https://github.com/llvm/llvm-project/pull/91879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Ignore try-statements in UnreachableCode checker (PR #91675)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Ignore try-statements in dead code checker (PR #91675)

[Balazs Benics via cfe-commits]

https://github.com/steakhal updated 
https://github.com/llvm/llvm-project/pull/91675

>From 846be0552bd2da608fc1729e5928d85650e1ce06 Mon Sep 17 00:00:00 2001
From: Andrew Sukach 
Date: Thu, 9 May 2024 18:49:41 -0400
Subject: [PATCH 1/3] [clang][static analyzer] ignore try statements in dead
 code checker

---
 .../Checkers/UnreachableCodeChecker.cpp  |  4 +++-
 clang/test/Analysis/unreachable-code-exceptions.cpp  | 12 
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/Analysis/unreachable-code-exceptions.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
index d24a124f5ffee..7ce9a5b5bb6dc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -159,6 +159,8 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph 
,
   SL = DL.asLocation();
   if (SR.isInvalid() || !SL.isValid())
 continue;
+  if (isa(S))
+continue;
 }
 else
   continue;
@@ -254,4 +256,4 @@ void ento::registerUnreachableCodeChecker(CheckerManager 
) {
 
 bool ento::shouldRegisterUnreachableCodeChecker(const CheckerManager ) {
   return true;
-}
+}
\ No newline at end of file
diff --git a/clang/test/Analysis/unreachable-code-exceptions.cpp 
b/clang/test/Analysis/unreachable-code-exceptions.cpp
new file mode 100644
index 0..aad7625b92d71
--- /dev/null
+++ b/clang/test/Analysis/unreachable-code-exceptions.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core -analyzer-checker=alpha.deadcode.UnreachableCode
+
+// expected-no-diagnostics
+
+void foo();
+
+void f4() {
+  try {
+foo();
+  } catch (int) {
+  }
+}
\ No newline at end of file

>From bfe9a244ed3e81669824f6d5a4e8b6ed9409375c Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Tue, 14 May 2024 08:55:29 +0200
Subject: [PATCH 2/3] Simplify RUN line.

---
 clang/test/Analysis/unreachable-code-exceptions.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/Analysis/unreachable-code-exceptions.cpp 
b/clang/test/Analysis/unreachable-code-exceptions.cpp
index aad7625b92d71..68c32e8a6e400 100644
--- a/clang/test/Analysis/unreachable-code-exceptions.cpp
+++ b/clang/test/Analysis/unreachable-code-exceptions.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core -analyzer-checker=alpha.deadcode.UnreachableCode
+// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core,alpha.deadcode.UnreachableCode
 
 // expected-no-diagnostics
 

>From 7531dc664b397b81c9ed6db87453d1c2fc52d878 Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Tue, 14 May 2024 08:55:45 +0200
Subject: [PATCH 3/3] Clarify the test

---
 clang/test/Analysis/unreachable-code-exceptions.cpp | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/clang/test/Analysis/unreachable-code-exceptions.cpp 
b/clang/test/Analysis/unreachable-code-exceptions.cpp
index 68c32e8a6e400..f47674ea8097d 100644
--- a/clang/test/Analysis/unreachable-code-exceptions.cpp
+++ b/clang/test/Analysis/unreachable-code-exceptions.cpp
@@ -4,9 +4,10 @@
 
 void foo();
 
-void f4() {
-  try {
+void fp_90162() {
+  try { // no-warning: The TryStmt shouldn't be unreachable.
 foo();
   } catch (int) {
+foo(); // We assume that catch handlers are reachable.
   }
-}
\ No newline at end of file
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Ignore try-statements in dead code checker (PR #91675)

[Balazs Benics via cfe-commits]

https://github.com/steakhal updated 
https://github.com/llvm/llvm-project/pull/91675

>From 846be0552bd2da608fc1729e5928d85650e1ce06 Mon Sep 17 00:00:00 2001
From: Andrew Sukach 
Date: Thu, 9 May 2024 18:49:41 -0400
Subject: [PATCH 1/2] [clang][static analyzer] ignore try statements in dead
 code checker

---
 .../Checkers/UnreachableCodeChecker.cpp  |  4 +++-
 clang/test/Analysis/unreachable-code-exceptions.cpp  | 12 
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/Analysis/unreachable-code-exceptions.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
index d24a124f5ffee..7ce9a5b5bb6dc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -159,6 +159,8 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph 
,
   SL = DL.asLocation();
   if (SR.isInvalid() || !SL.isValid())
 continue;
+  if (isa(S))
+continue;
 }
 else
   continue;
@@ -254,4 +256,4 @@ void ento::registerUnreachableCodeChecker(CheckerManager 
) {
 
 bool ento::shouldRegisterUnreachableCodeChecker(const CheckerManager ) {
   return true;
-}
+}
\ No newline at end of file
diff --git a/clang/test/Analysis/unreachable-code-exceptions.cpp 
b/clang/test/Analysis/unreachable-code-exceptions.cpp
new file mode 100644
index 0..aad7625b92d71
--- /dev/null
+++ b/clang/test/Analysis/unreachable-code-exceptions.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core -analyzer-checker=alpha.deadcode.UnreachableCode
+
+// expected-no-diagnostics
+
+void foo();
+
+void f4() {
+  try {
+foo();
+  } catch (int) {
+  }
+}
\ No newline at end of file

>From bfe9a244ed3e81669824f6d5a4e8b6ed9409375c Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Tue, 14 May 2024 08:55:29 +0200
Subject: [PATCH 2/2] Simplify RUN line.

---
 clang/test/Analysis/unreachable-code-exceptions.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/Analysis/unreachable-code-exceptions.cpp 
b/clang/test/Analysis/unreachable-code-exceptions.cpp
index aad7625b92d71..68c32e8a6e400 100644
--- a/clang/test/Analysis/unreachable-code-exceptions.cpp
+++ b/clang/test/Analysis/unreachable-code-exceptions.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core -analyzer-checker=alpha.deadcode.UnreachableCode
+// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core,alpha.deadcode.UnreachableCode
 
 // expected-no-diagnostics
 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Ignore try-statements in dead code checker (PR #91675)

[Balazs Benics via cfe-commits]

@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core -analyzer-checker=alpha.deadcode.UnreachableCode

steakhal wrote:

```suggestion
// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core,alpha.deadcode.UnreachableCode
```

https://github.com/llvm/llvm-project/pull/91675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Ignore try-statements in dead code checker (PR #91675)

[Balazs Benics via cfe-commits]

@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core -analyzer-checker=alpha.deadcode.UnreachableCode
+
+// expected-no-diagnostics
+
+void foo();
+
+void f4() {
+  try {
+foo();
+  } catch (int) {
+  }
+}

steakhal wrote:

```suggestion
void fp_90162() {
  try { // no-warning: The TryStmt shouldn't be unreachable.
foo();
  } catch (int) {
foo(); // We assume that catch handlers are reachable.
  }
}

```

https://github.com/llvm/llvm-project/pull/91675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Ignore try-statements in dead code checker (PR #91675)

[Balazs Benics via cfe-commits]

https://github.com/steakhal approved this pull request.


https://github.com/llvm/llvm-project/pull/91675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Ignore try-statements in dead code checker (PR #91675)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Ignore try-statements in dead code checker (PR #91675)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][static analyzer] ignore try statements in dead code checker (PR #91675)

[Balazs Benics via cfe-commits]

https://github.com/steakhal requested changes to this pull request.

Please remove the formatting changes. (Keep only the relevant lines)
Add a test demonstrating that this fixes a false-positive.

To find which file you need to add your test, I'd recommend braking something 
inside the checker to see which tests break. Then pick the file that seems to 
be the most relevant.

https://github.com/llvm/llvm-project/pull/91675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Move `CTUPhase1InliningMode` option to String analyzer options category (PR #91932)

[Balazs Benics via cfe-commits]

https://github.com/steakhal closed 
https://github.com/llvm/llvm-project/pull/91932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Move `CTUPhase1InliningMode` option to String analyzer options category (PR #91932)

[Balazs Benics via cfe-commits]

https://github.com/steakhal approved this pull request.

Fair!

https://github.com/llvm/llvm-project/pull/91932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Move `CTUPhase1InliningMode` option to String analyzer options category (PR #91932)

[Balazs Benics via cfe-commits]

https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -1179,6 +1179,34 @@ security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C)
strncpy(buf, "a", 1); // warn
  }
 
+security.SetgidSetuidOrder (C)
+""

steakhal wrote:

I don't have a preference, I was just asking.
I just felt this is a natural question, and you seem to care about alpha 
checkers and documentation/discoverability, and this seemed like a subject to 
discuss. @NagyDonat WDYT?

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/steakhal edited 
https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,185 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,security.SetgidSetuidOrder 
-verify %s
+
+typedef int uid_t;
+typedef int gid_t;
+
+int setuid(uid_t);
+int setgid(gid_t);
+int seteuid(uid_t);
+int setegid(gid_t);
+int setreuid(uid_t, uid_t);
+int setregid(gid_t, gid_t);
+int setresuid(uid_t, uid_t, uid_t);
+int setresgid(gid_t, gid_t, gid_t);
+
+uid_t getuid();
+gid_t getgid();
+
+
+
+void correct_order() {
+  if (setgid(getgid()) == -1)
+return;
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void incorrect_order() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void warn_at_second_time() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+}
+
+uid_t f_uid();
+gid_t f_gid();
+
+void setuid_other() {
+  if (setuid(f_uid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setgid_other() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(f_gid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setuid_other_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setuid(f_uid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setgid_with_getuid() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getuid()) == -1)
+return;
+}
+
+void setuid_with_getgid() {
+  if (setuid(getgid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+int f_setuid() {
+  return setuid(getuid());
+}
+
+int f_setgid() {
+  return setgid(getgid()); // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+}
+
+void function_calls() {
+  if (f_setuid() == -1)
+return;
+  if (f_setgid() == -1)
+return;
+}
+
+void seteuid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (seteuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setegid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setegid(getgid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setreuid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setreuid(getuid(), getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setregid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setregid(getgid(), getgid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setresuid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setresuid(getuid(), getuid(), getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setresgid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setresgid(getgid(), getgid(), getgid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void other_system_function_between() {
+  if (setuid(getuid()) == -1)
+return;
+  gid_t g = getgid();

steakhal wrote:

Did you mean to use `g` on the next line? If not, then have you considered just 
casting this to void instead of introducing a new variable?

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -1179,6 +1179,34 @@ security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C)
strncpy(buf, "a", 1); // warn
  }
 
+security.SetgidSetuidOrder (C)
+""

steakhal wrote:

Give that the 
[`alpha.unix.chroot`](https://clang.llvm.org/docs/analyzer/checkers.html#alpha-unix-chroot-c)
 checker does something similar, I wonder if this checker should share the same 
parent package with that one to aid discoverability.
WDYT?

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent ,
+   CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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 ,
+ CheckerContext ) 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 (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});
+  C.addTransition(State, C.getPredecessor());
+}
+
+ProgramStateRef SetgidSetuidOrderChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+   

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{

steakhal wrote:

Why is this field not constant, like the others?

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}

steakhal wrote:

Couldn't we just omit this, and leave the compiler generate this for us?

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,185 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,security.SetgidSetuidOrder 
-verify %s
+
+typedef int uid_t;
+typedef int gid_t;
+
+int setuid(uid_t);
+int setgid(gid_t);
+int seteuid(uid_t);
+int setegid(gid_t);
+int setreuid(uid_t, uid_t);
+int setregid(gid_t, gid_t);
+int setresuid(uid_t, uid_t, uid_t);
+int setresgid(gid_t, gid_t, gid_t);
+
+uid_t getuid();
+gid_t getgid();
+
+
+
+void correct_order() {
+  if (setgid(getgid()) == -1)
+return;
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void incorrect_order() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void warn_at_second_time() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+}
+
+uid_t f_uid();
+gid_t f_gid();
+
+void setuid_other() {
+  if (setuid(f_uid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setgid_other() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(f_gid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setuid_other_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setuid(f_uid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setgid_with_getuid() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getuid()) == -1)
+return;
+}
+
+void setuid_with_getgid() {
+  if (setuid(getgid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+int f_setuid() {
+  return setuid(getuid());
+}
+
+int f_setgid() {
+  return setgid(getgid()); // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+}
+
+void function_calls() {
+  if (f_setuid() == -1)
+return;
+  if (f_setgid() == -1)
+return;
+}
+
+void seteuid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (seteuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setegid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setegid(getgid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setreuid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setreuid(getuid(), getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setregid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setregid(getgid(), getgid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setresuid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setresuid(getuid(), getuid(), getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setresgid_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setresgid(getgid(), getgid(), getgid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void other_system_function_between() {
+  if (setuid(getuid()) == -1)
+return;
+  gid_t g = getgid();
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+}
+
+void f_extern();
+
+void other_unknown_function_between() {
+  if (setuid(getuid()) == -1)
+return;
+  f_extern();
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}

steakhal wrote:

I was thinking about this for a minute. I think it's fine, but I'll leave here 
my thought anyway:
Technically, `f_extern` could have already drop the privileges. But I get it 
that would imply that this checker is basically useless after the first opaque 
call - which is not practical.

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent ,
+   CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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 ,
+ CheckerContext ) 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 (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});

steakhal wrote:

```suggestion
  State = State->set(nullptr);
```

This comes up a few times in this patch. Personally, I prefer to explicitly say 
"nullptr" is something is that - and I'm not writing 

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent ,
+   CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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 ,
+ CheckerContext ) 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 (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});
+  C.addTransition(State, C.getPredecessor());
+}
+
+ProgramStateRef SetgidSetuidOrderChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+   

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,185 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,security.SetgidSetuidOrder 
-verify %s
+
+typedef int uid_t;
+typedef int gid_t;
+
+int setuid(uid_t);
+int setgid(gid_t);
+int seteuid(uid_t);
+int setegid(gid_t);
+int setreuid(uid_t, uid_t);
+int setregid(gid_t, gid_t);
+int setresuid(uid_t, uid_t, uid_t);
+int setresgid(gid_t, gid_t, gid_t);
+
+uid_t getuid();
+gid_t getgid();
+
+
+
+void correct_order() {
+  if (setgid(getgid()) == -1)
+return;
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void incorrect_order() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void warn_at_second_time() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}

steakhal wrote:

I'm not sure of the value proposition with this second warning.

To me, the code has a sequence (the middle 2 calls), that should satisfy the 
CERT rule.
After seeing a "good" pattern, I'd expect the checker to ignore the rest, as we 
already dropped the privileges, right?

So the question is: is it valuable to report this?

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent ,
+   CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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 ,
+ CheckerContext ) 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 (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});
+  C.addTransition(State, C.getPredecessor());
+}
+
+ProgramStateRef SetgidSetuidOrderChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+   

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,170 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,security.SetgidSetuidOrder 
-verify %s
+
+#include "Inputs/system-header-simulator-setgid-setuid.h"
+
+void correct_order() {
+  if (setgid(getgid()) == -1)
+return;
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void incorrect_order() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void warn_at_second_time() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call 
following a 'setuid(getuid())' call is likely to fail}}
+return;
+}
+
+uid_t f_uid();
+gid_t f_gid();
+
+void setuid_other() {
+  if (setuid(f_uid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setgid_other() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(f_gid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setuid_other_between() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setuid(f_uid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}
+
+void setgid_with_getuid() {
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getuid()) == -1)
+return;
+}
+
+void setuid_with_getgid() {
+  if (setuid(getgid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;
+}

steakhal wrote:

Tidy would a nice fit, yes.

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent ,
+   CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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 ,
+ CheckerContext ) 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 (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});
+  C.addTransition(State, C.getPredecessor());
+}
+
+ProgramStateRef SetgidSetuidOrderChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+   

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,170 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,security.SetgidSetuidOrder 
-verify %s
+
+#include "Inputs/system-header-simulator-setgid-setuid.h"
+
+void correct_order() {
+  if (setgid(getgid()) == -1)
+return;
+  if (setuid(getuid()) == -1)
+return;
+  if (setgid(getgid()) == -1)
+return;

steakhal wrote:

I also find it weird that we start the test file with an edge-case :D

https://github.com/llvm/llvm-project/pull/91445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent ,
+   CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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 ,
+ CheckerContext ) 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 (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});
+  C.addTransition(State, C.getPredecessor());
+}
+
+ProgramStateRef SetgidSetuidOrderChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+   

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent ,
+   CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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 ,
+ CheckerContext ) 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 (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});
+  C.addTransition(State, C.getPredecessor());

steakhal wrote:

```suggestion
  C.addTransition(State);
```

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

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- 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 {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent , CheckerContext ) const;
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent ,
+CheckerContext ) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent ,
+   CheckerContext ) 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(const CallDescription ,
+ const CallEvent ) const;
+  void emitReport(ProgramStateRef State, CheckerContext ) const;
+};
+
+} // 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 ,
+ CheckerContext ) 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 (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});
+  C.addTransition(State, C.getPredecessor());
+}
+
+ProgramStateRef SetgidSetuidOrderChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+   

[clang] [clang][analyzer] Check for label location bindings in `DereferenceChecker` (PR #91119)

[Balazs Benics via cfe-commits]

https://github.com/steakhal approved this pull request.


https://github.com/llvm/llvm-project/pull/91119
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Check for label location bindings in `DereferenceChecker` (PR #91119)

[Balazs Benics via cfe-commits]

steakhal wrote:

This one looks good to me. I wanna hear your opinion @NagyDonat 

https://github.com/llvm/llvm-project/pull/91119
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits