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