diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.qhelp b/python/ql/src/Security/CWE-285/PamAuthorization.qhelp similarity index 78% rename from python/ql/src/experimental/Security/CWE-285/PamAuthorization.qhelp rename to python/ql/src/Security/CWE-285/PamAuthorization.qhelp index 8e0f829f33ee..63af4c2f22df 100644 --- a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.qhelp +++ b/python/ql/src/Security/CWE-285/PamAuthorization.qhelp @@ -9,7 +9,9 @@

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. + 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 an expired + login or a password can still access the system.

@@ -26,7 +28,9 @@

- 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 + In the following example, the code only checks the credentials of a user. Hence, + in this case, a user with expired credentials 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.

@@ -46,4 +50,4 @@ pam_acct_mgmt - \ No newline at end of file + diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql b/python/ql/src/Security/CWE-285/PamAuthorization.ql similarity index 75% rename from python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql rename to python/ql/src/Security/CWE-285/PamAuthorization.ql index 67be2fdd8ab5..233e42e62394 100644 --- a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql +++ b/python/ql/src/Security/CWE-285/PamAuthorization.ql @@ -1,8 +1,9 @@ /** - * @name Authorization bypass due to incorrect usage of PAM - * @description Using only the `pam_authenticate` call to check the validity of a login can lead to a authorization bypass. + * @name PAM authorization bypass due to incorrect usage + * @description Not using `pam_acct_mgmt` after `pam_authenticate` to check the validity of a login can lead to authorization bypass. * @kind problem * @problem.severity warning + * @security-severity 8.1 * @precision high * @id py/pam-auth-bypass * @tags security @@ -33,4 +34,5 @@ where acctMgmtCall = libPam().getMember("pam_acct_mgmt").getACall() and DataFlow::localFlow(handle, acctMgmtCall.getArg(0)) ) -select authenticateCall, "This PAM authentication call may be lead to an authorization bypass." +select authenticateCall, + "This PAM authentication call may lead to an authorization bypass, since 'pam_acct_mgmt' is not called afterwards." diff --git a/python/ql/src/Security/CWE-285/PamAuthorizationBad.py b/python/ql/src/Security/CWE-285/PamAuthorizationBad.py new file mode 100644 index 000000000000..15e952ab6b28 --- /dev/null +++ b/python/ql/src/Security/CWE-285/PamAuthorizationBad.py @@ -0,0 +1,19 @@ +libpam = CDLL(find_library("pam")) + +pam_authenticate = libpam.pam_authenticate +pam_authenticate.restype = c_int +pam_authenticate.argtypes = [PamHandle, c_int] + +def authenticate(username, password, service='login'): + def my_conv(n_messages, messages, p_response, app_data): + """ + Simple conversation function that responds to any prompt where the echo is off with the supplied password + """ + ... + + handle = PamHandle() + conv = PamConv(my_conv, 0) + retval = pam_start(service, username, byref(conv), byref(handle)) + + retval = pam_authenticate(handle, 0) + return retval == 0 diff --git a/python/ql/src/Security/CWE-285/PamAuthorizationGood.py b/python/ql/src/Security/CWE-285/PamAuthorizationGood.py new file mode 100644 index 000000000000..eecb5b94a0e6 --- /dev/null +++ b/python/ql/src/Security/CWE-285/PamAuthorizationGood.py @@ -0,0 +1,25 @@ +libpam = CDLL(find_library("pam")) + +pam_authenticate = libpam.pam_authenticate +pam_authenticate.restype = c_int +pam_authenticate.argtypes = [PamHandle, c_int] + +pam_acct_mgmt = libpam.pam_acct_mgmt +pam_acct_mgmt.restype = c_int +pam_acct_mgmt.argtypes = [PamHandle, c_int] + +def authenticate(username, password, service='login'): + def my_conv(n_messages, messages, p_response, app_data): + """ + Simple conversation function that responds to any prompt where the echo is off with the supplied password + """ + ... + + handle = PamHandle() + conv = PamConv(my_conv, 0) + retval = pam_start(service, username, byref(conv), byref(handle)) + + retval = pam_authenticate(handle, 0) + if retval == 0: + retval = pam_acct_mgmt(handle, 0) + return retval == 0 diff --git a/python/ql/src/change-notes/2022-05-10-promote-pam-authentication-bypass.md b/python/ql/src/change-notes/2022-05-10-promote-pam-authentication-bypass.md new file mode 100644 index 000000000000..e87e717f6099 --- /dev/null +++ b/python/ql/src/change-notes/2022-05-10-promote-pam-authentication-bypass.md @@ -0,0 +1,4 @@ +--- +category: newQuery +--- +* The query "PAM authorization bypass due to incorrect usage" (`py/pam-auth-bypass`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @porcupineyhairs](https://github.com/github/codeql/pull/8595). diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py b/python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py deleted file mode 100644 index 257f9b997292..000000000000 --- a/python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py +++ /dev/null @@ -1,13 +0,0 @@ -def authenticate(self, username, password, service='login', encoding='utf-8', resetcreds=True): - libpam = CDLL(find_library("pam")) - pam_authenticate = libpam.pam_authenticate - pam_authenticate.restype = c_int - pam_authenticate.argtypes = [PamHandle, c_int] - - - handle = PamHandle() - conv = PamConv(my_conv, 0) - retval = pam_start(service, username, byref(conv), byref(handle)) - - retval = pam_authenticate(handle, 0) - return retval == 0 \ No newline at end of file diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorizationGood.py b/python/ql/src/experimental/Security/CWE-285/PamAuthorizationGood.py deleted file mode 100644 index 0f047c6ac659..000000000000 --- a/python/ql/src/experimental/Security/CWE-285/PamAuthorizationGood.py +++ /dev/null @@ -1,17 +0,0 @@ -def authenticate(self, username, password, service='login', encoding='utf-8', resetcreds=True): - libpam = CDLL(find_library("pam")) - pam_authenticate = libpam.pam_authenticate - pam_acct_mgmt = libpam.pam_acct_mgmt - pam_authenticate.restype = c_int - pam_authenticate.argtypes = [PamHandle, c_int] - pam_acct_mgmt.restype = c_int - pam_acct_mgmt.argtypes = [PamHandle, c_int] - - handle = PamHandle() - conv = PamConv(my_conv, 0) - retval = pam_start(service, username, byref(conv), byref(handle)) - - retval = pam_authenticate(handle, 0) - if retval == 0: - retval = pam_acct_mgmt(handle, 0) - return retval == 0 \ No newline at end of file diff --git a/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected b/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected deleted file mode 100644 index 1b6c23291be9..000000000000 --- a/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected +++ /dev/null @@ -1 +0,0 @@ -| pam_test.py:48:18:48:44 | ControlFlowNode for pam_authenticate() | This PAM authentication call may be lead to an authorization bypass. | diff --git a/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.qlref b/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.qlref deleted file mode 100644 index 38fac298b1e3..000000000000 --- a/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE-285/PamAuthorization.ql diff --git a/python/ql/test/query-tests/Security/CWE-285-PamAuthorization/PamAuthorization.expected b/python/ql/test/query-tests/Security/CWE-285-PamAuthorization/PamAuthorization.expected new file mode 100644 index 000000000000..54b38f21aa67 --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-285-PamAuthorization/PamAuthorization.expected @@ -0,0 +1 @@ +| pam_test.py:48:18:48:44 | ControlFlowNode for pam_authenticate() | This PAM authentication call may lead to an authorization bypass, since 'pam_acct_mgmt' is not called afterwards. | diff --git a/python/ql/test/query-tests/Security/CWE-285-PamAuthorization/PamAuthorization.qlref b/python/ql/test/query-tests/Security/CWE-285-PamAuthorization/PamAuthorization.qlref new file mode 100644 index 000000000000..81915461d7ad --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-285-PamAuthorization/PamAuthorization.qlref @@ -0,0 +1 @@ +Security/CWE-285/PamAuthorization.ql diff --git a/python/ql/test/experimental/query-tests/Security/CWE-285/pam_test.py b/python/ql/test/query-tests/Security/CWE-285-PamAuthorization/pam_test.py similarity index 100% rename from python/ql/test/experimental/query-tests/Security/CWE-285/pam_test.py rename to python/ql/test/query-tests/Security/CWE-285-PamAuthorization/pam_test.py