diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorization.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorization.qhelp new file mode 100644 index 000000000000..debf30dfa65d --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorization.qhelp @@ -0,0 +1,49 @@ + + + +

+ Using only a call to + pam_authenticate + to check the validity of a login can lead to authorization bypass vulnerabilities. +

+

+ A + pam_authenticate + 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. +

+ +
+ + +

+ A call to + pam_authenticate + should be followed by a call to + pam_acct_mgmt + to check if a user is allowed to login. +

+
+ + +

+ 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 + chage -E0 `username` + and then trying to log in. +

+ + +

+ This can be avoided by calling + pam_acct_mgmt + call to verify access as has been done in the snippet shown below. +

+ +
+ + +
  • + Man-Page: + pam_acct_mgmt +
  • +
    +
    \ No newline at end of file diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorization.ql b/cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorization.ql new file mode 100644 index 000000000000..5292a705d93b --- /dev/null +++ b/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." diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorizationBad.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorizationBad.cpp new file mode 100644 index 000000000000..cde024cde343 --- /dev/null +++ b/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; +} diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorizationGood.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-285/PamAuthorizationGood.cpp new file mode 100644 index 000000000000..e4a2ce008c54 --- /dev/null +++ b/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; +} diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/PamAuthorization.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/PamAuthorization.expected new file mode 100644 index 000000000000..af34f888df2e --- /dev/null +++ b/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. | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/PamAuthorization.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/PamAuthorization.qlref new file mode 100644 index 000000000000..f1135f7d536a --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/PamAuthorization.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-285/PamAuthorization.ql diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-285/test.cpp new file mode 100644 index 000000000000..e2753f10775e --- /dev/null +++ b/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; +}