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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= <balazs.keri at ericsson.com>,
=?utf-8?q?Balázs_Kéri?= <balazs.keri at ericsson.com>,
=?utf-8?q?Balázs_Kéri?= <balazs.keri at ericsson.com>,
=?utf-8?q?Balázs_Kéri?= <balazs.keri at ericsson.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/91445 at github.com>


================
@@ -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