Merge pull request #8775 from porcupineyhairs/cpam

CPP: PAM Authorization Bypass

Author: geoffw0

cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorization.qhelp

@@ -0,0 +1,49 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+      Using only a call to
+      <code>pam_authenticate</code>
+      to check the validity of a login can lead to authorization bypass vulnerabilities.
+    </p>
+    <p>
+      A
+      <code>pam_authenticate</code>
+      only verifies the credentials of a user. It does not check if a user has an appropriate authorization to actually login. This means a user with a expired login or a password can still access the system.
+    </p>
+
+  </overview>
+
+  <recommendation>
+    <p>
+      A call to
+      <code>pam_authenticate</code>
+      should be followed by a call to
+      <code>pam_acct_mgmt</code>
+      to check if a user is allowed to login.
+    </p>
+  </recommendation>
+
+  <example>
+    <p>
+      In the following example, the code only checks the credentials of a user. Hence, in this case, a user expired with expired creds can still login. This can be verified by creating a new user account, expiring it with
+      <code>chage -E0 `username` </code>
+      and then trying to log in.
+    </p>
+    <sample src="PamAuthorizationBad.cpp" />
+
+    <p>
+      This can be avoided by calling
+      <code>pam_acct_mgmt</code>
+      call to verify access as has been done in the snippet shown below.
+    </p>
+    <sample src="PamAuthorizationGood.cpp" />
+  </example>
+
+  <references>
+    <li>
+      Man-Page:
+      <a href="https://man7.org/linux/man-pages/man3/pam_acct_mgmt.3.html">pam_acct_mgmt</a>
+    </li>
+  </references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorization.ql

@@ -0,0 +1,34 @@
+/**
+ * @name PAM Authorization bypass
+ * @description Only using `pam_authenticate` call to authenticate users can lead to authorization vulnerabilities.
+ * @kind problem
+ * @problem.severity error
+ * @id cpp/pam-auth-bypass
+ * @tags security
+ *       external/cwe/cwe-285
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+private class PamAuthCall extends FunctionCall {
+  PamAuthCall() {
+    exists(Function f | f.hasName("pam_authenticate") | f.getACallToThisFunction() = this)
+  }
+}
+
+private class PamActMgmtCall extends FunctionCall {
+  PamActMgmtCall() {
+    exists(Function f | f.hasName("pam_acct_mgmt") | f.getACallToThisFunction() = this)
+  }
+}
+
+from PamAuthCall pa, Expr handle
+where
+  pa.getArgument(0) = handle and
+  not exists(PamActMgmtCall pac |
+    globalValueNumber(handle) = globalValueNumber(pac.getArgument(0)) or
+    DataFlow::localExprFlow(handle, pac.getArgument(0))
+  )
+select pa, "This PAM authentication call may be lead to an authorization bypass."

cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorizationBad.cpp

@@ -0,0 +1,20 @@
+bool PamAuthGood(const std::string &username_in,
+                 const std::string &password_in,
+                 std::string &authenticated_username)
+{
+
+    struct pam_handle *pamh = nullptr; /* pam session handle */
+
+    const char *username = username_in.c_str();
+    int err = pam_start("test", username,
+                        0, &pamh);
+    if (err != PAM_SUCCESS)
+    {
+        return false;
+    }
+
+    err = pam_authenticate(pamh, 0); // BAD
+    if (err != PAM_SUCCESS)
+        return err;
+    return true;
+}

cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorizationGood.cpp

@@ -0,0 +1,24 @@
+bool PamAuthGood(const std::string &username_in,
+                 const std::string &password_in,
+                 std::string &authenticated_username)
+{
+
+    struct pam_handle *pamh = nullptr; /* pam session handle */
+
+    const char *username = username_in.c_str();
+    int err = pam_start("test", username,
+                        0, &pamh);
+    if (err != PAM_SUCCESS)
+    {
+        return false;
+    }
+
+    err = pam_authenticate(pamh, 0);
+    if (err != PAM_SUCCESS)
+        return err;
+
+    err = pam_acct_mgmt(pamh, 0); // GOOD
+    if (err != PAM_SUCCESS)
+        return err;
+    return true;
+}

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/PamAuthorization.expected

@@ -0,0 +1 @@
+| test.cpp:29:11:29:26 | call to pam_authenticate | This PAM authentication call may be lead to an authorization bypass. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/PamAuthorization.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-285/PamAuthorization.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/test.cpp

@@ -0,0 +1,59 @@
+#include "../../../../../library-tests/dataflow/taint-tests/stl.h"
+
+using namespace std;
+
+#define PAM_SUCCESS 1
+
+typedef struct pam_handle
+{
+};
+int pam_start(std::string servicename, std::string username, int a, struct pam_handle **);
+int pam_authenticate(struct pam_handle *, int e);
+int pam_acct_mgmt(struct pam_handle *, int e);
+
+bool PamAuthBad(const std::string &username_in,
+                const std::string &password_in,
+                std::string &authenticated_username)
+{
+
+    struct pam_handle *pamh = nullptr; /* pam session handle */
+
+    const char *username = username_in.c_str();
+    int err = pam_start("test", username,
+                        0, &pamh);
+    if (err != PAM_SUCCESS)
+    {
+        return false;
+    }
+
+    err = pam_authenticate(pamh, 0);
+    if (err != PAM_SUCCESS)
+        return err;
+
+    return true;
+}
+
+bool PamAuthGood(const std::string &username_in,
+                 const std::string &password_in,
+                 std::string &authenticated_username)
+{
+
+    struct pam_handle *pamh = nullptr; /* pam session handle */
+
+    const char *username = username_in.c_str();
+    int err = pam_start("test", username,
+                        0, &pamh);
+    if (err != PAM_SUCCESS)
+    {
+        return false;
+    }
+
+    err = pam_authenticate(pamh, 0);
+    if (err != PAM_SUCCESS)
+        return err;
+
+    err = pam_acct_mgmt(pamh, 0);
+    if (err != PAM_SUCCESS)
+        return err;
+    return true;
+}