Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
<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.
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.
</p>

</overview>
Expand All @@ -26,7 +28,9 @@

<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
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
<code>chage -E0 `username` </code>
and then trying to log in.
</p>
Expand All @@ -46,4 +50,4 @@
<a href="https://man7.org/linux/man-pages/man3/pam_acct_mgmt.3.html">pam_acct_mgmt</a>
</li>
</references>
</qhelp>
</qhelp>
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -33,4 +34,5 @@ where
acctMgmtCall = libPam().getMember("pam_acct_mgmt").getACall() and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not allowed to add a comment in the appropriate place, but there's a use of API::moduleImport("ctypes.util") on line 20 above that will need fixing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, that probably what the merge conflict is also about 👍

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."
19 changes: 19 additions & 0 deletions python/ql/src/Security/CWE-285/PamAuthorizationBad.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
libpam = CDLL(find_library("pam"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's... Quite the formatting. 😮


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
25 changes: 25 additions & 0 deletions python/ql/src/Security/CWE-285/PamAuthorizationGood.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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).

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Security/CWE-285/PamAuthorization.ql