Python : Add query to detect PAM authorization bypass#8595
Conversation
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. This PR includes a qhelp describing the issue, a query which detects instances where a call to `pam_acc_mgmt` does not follow a call to `pam_authenticate` and it's corresponding tests. This PR has multiple detections. Some of the public one I can find are : * [CVE-2022-0860](https://nvd.nist.gov/vuln/detail/CVE-2022-0860) found in [cobbler/cobbler](https://www.github.com/cobbler/cobbler) * [fredhutch/motuz](https://www.huntr.dev/bounties/d46f91ca-b8ef-4b67-a79a-2420c4c6d52b/)
|
@RasmusWL This should ideally also find CVE-2022-1049 found in clusterlabs/pcs and fixed in ClusterLabs/pcs@fb86000#diff-e8d00a7c356632c11156ad3e8ce897f66538c4cc4a849487f9a5ddc0a19eafb8L53 However, CodeQL does not seem to be able to trace the flow from the |
|
Thanks for this submission 👍 I'll take a look at it either tomorrow or next week. |
RasmusWL
left a comment
There was a problem hiding this comment.
Besides inline comments, I would like you to merge the test files to a single, and remove all the stuff that is not absolutely required to make the example work (I highlight a bit of it, but I think there are more things that can be removed).
python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql
Outdated
Show resolved
Hide resolved
python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql
Outdated
Show resolved
Hide resolved
python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py
Outdated
Show resolved
Hide resolved
python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py
Outdated
Show resolved
Hide resolved
python/ql/test/experimental/query-tests/Security/CWE-285/good.py
Outdated
Show resolved
Hide resolved
python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql
Outdated
Show resolved
Hide resolved
python/ql/test/experimental/query-tests/Security/CWE-285/good.py
Outdated
Show resolved
Hide resolved
python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql
Outdated
Show resolved
Hide resolved
|
@RasmusWL Thanks for the quick response. I have made the changes you sought. Any thoughts on how I could handle the |
Thanks 🙏 I made an other minor adjustment, simplifying the code a little bit more. Hope you don't mind.
This is sadly a known problem, which is highlighted by this bit of test code. In the code below, a limitation from type trackers is that we don't treat def id(obj):
return obj
x = foo()
also_x = id(x) |
|
@RasmusWL Ping. |
|
Thanks. I've just gotten back from vacation. Need to do a little internal testing of this before merging. |
RasmusWL
left a comment
There was a problem hiding this comment.
Had a chance to look at some results, which looks good to me, so added @precision high. Will merge this PR once tests turn green.
Using only a call to
pam_authenticateto check the validity of a login can lead to authorization bypass vulnerabilities. Apam_authenticateonly 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.This PR includes a qhelp describing the issue, a query which detects instances where a call to
pam_acc_mgmtdoes not follow a call topam_authenticateand it's corresponding tests.Some of the public CVE's I can find using this query are :