Skip to content

Commit 3745526

Browse files
authored
Merge pull request #9108 from RasmusWL/promote-pam
Python: Promote `py/pam-auth-bypass`
2 parents 04ca9cf + 6611e5b commit 3745526

File tree

12 files changed

+62
-38
lines changed

12 files changed

+62
-38
lines changed

python/ql/src/experimental/Security/CWE-285/PamAuthorization.qhelp renamed to python/ql/src/Security/CWE-285/PamAuthorization.qhelp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
<p>
1010
A
1111
<code>pam_authenticate</code>
12-
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.
12+
only verifies the credentials of a user. It does not check if a user has an
13+
appropriate authorization to actually login. This means a user with an expired
14+
login or a password can still access the system.
1315
</p>
1416

1517
</overview>
@@ -26,7 +28,9 @@
2628

2729
<example>
2830
<p>
29-
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
31+
In the following example, the code only checks the credentials of a user. Hence,
32+
in this case, a user with expired credentials can still login. This can be
33+
verified by creating a new user account, expiring it with
3034
<code>chage -E0 `username` </code>
3135
and then trying to log in.
3236
</p>
@@ -46,4 +50,4 @@
4650
<a href="https://man7.org/linux/man-pages/man3/pam_acct_mgmt.3.html">pam_acct_mgmt</a>
4751
</li>
4852
</references>
49-
</qhelp>
53+
</qhelp>

python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql renamed to python/ql/src/Security/CWE-285/PamAuthorization.ql

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
/**
2-
* @name Authorization bypass due to incorrect usage of PAM
3-
* @description Using only the `pam_authenticate` call to check the validity of a login can lead to a authorization bypass.
2+
* @name PAM authorization bypass due to incorrect usage
3+
* @description Not using `pam_acct_mgmt` after `pam_authenticate` to check the validity of a login can lead to authorization bypass.
44
* @kind problem
55
* @problem.severity warning
6+
* @security-severity 8.1
67
* @precision high
78
* @id py/pam-auth-bypass
89
* @tags security
@@ -33,4 +34,5 @@ where
3334
acctMgmtCall = libPam().getMember("pam_acct_mgmt").getACall() and
3435
DataFlow::localFlow(handle, acctMgmtCall.getArg(0))
3536
)
36-
select authenticateCall, "This PAM authentication call may be lead to an authorization bypass."
37+
select authenticateCall,
38+
"This PAM authentication call may lead to an authorization bypass, since 'pam_acct_mgmt' is not called afterwards."
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
libpam = CDLL(find_library("pam"))
2+
3+
pam_authenticate = libpam.pam_authenticate
4+
pam_authenticate.restype = c_int
5+
pam_authenticate.argtypes = [PamHandle, c_int]
6+
7+
def authenticate(username, password, service='login'):
8+
def my_conv(n_messages, messages, p_response, app_data):
9+
"""
10+
Simple conversation function that responds to any prompt where the echo is off with the supplied password
11+
"""
12+
...
13+
14+
handle = PamHandle()
15+
conv = PamConv(my_conv, 0)
16+
retval = pam_start(service, username, byref(conv), byref(handle))
17+
18+
retval = pam_authenticate(handle, 0)
19+
return retval == 0
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
libpam = CDLL(find_library("pam"))
2+
3+
pam_authenticate = libpam.pam_authenticate
4+
pam_authenticate.restype = c_int
5+
pam_authenticate.argtypes = [PamHandle, c_int]
6+
7+
pam_acct_mgmt = libpam.pam_acct_mgmt
8+
pam_acct_mgmt.restype = c_int
9+
pam_acct_mgmt.argtypes = [PamHandle, c_int]
10+
11+
def authenticate(username, password, service='login'):
12+
def my_conv(n_messages, messages, p_response, app_data):
13+
"""
14+
Simple conversation function that responds to any prompt where the echo is off with the supplied password
15+
"""
16+
...
17+
18+
handle = PamHandle()
19+
conv = PamConv(my_conv, 0)
20+
retval = pam_start(service, username, byref(conv), byref(handle))
21+
22+
retval = pam_authenticate(handle, 0)
23+
if retval == 0:
24+
retval = pam_acct_mgmt(handle, 0)
25+
return retval == 0
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* 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).

python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py

Lines changed: 0 additions & 13 deletions
This file was deleted.

python/ql/src/experimental/Security/CWE-285/PamAuthorizationGood.py

Lines changed: 0 additions & 17 deletions
This file was deleted.

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

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

Lines changed: 0 additions & 1 deletion
This file was deleted.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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. |

0 commit comments

Comments
 (0)