Skip to content

Python: Promote py/pam-auth-bypass - #9108

Merged
tausbn merged 9 commits into
github:mainfrom
RasmusWL:promote-pam
May 23, 2022
Merged

Python: Promote py/pam-auth-bypass#9108
tausbn merged 9 commits into
github:mainfrom
RasmusWL:promote-pam

Conversation

@RasmusWL

Copy link
Copy Markdown
Member

This promotes the experimental query from #8595

RasmusWL added 4 commits May 10, 2022 17:59
99% of our .qhelp files have manually wrapped lines, so just wanted to
keep things consistent
They did not have proper formatting (only 2 spaces), and I restructured
them a bit more so they look like code in the wild
Thought that calling out the actual vulnerability would make things
easier for our end users :)
@RasmusWL
RasmusWL requested a review from a team as a code owner May 10, 2022 16:07
Comment thread python/ql/src/Security/CWE-285/PamAuthorization.ql Fixed
The value 8.1 was calculated by our internal tool. This corresponds to a
'High' severity, which from my gut feeling seems reasonable for
authorization bypass.

@tausbn tausbn left a comment

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.

A few minor bits and bobs that need fixing (most of them presented as suggestions for easy inclusion), but otherwise this looks good to me. 👍

Comment thread python/ql/src/Security/CWE-285/PamAuthorization.qhelp Outdated
Comment thread python/ql/src/Security/CWE-285/PamAuthorization.qhelp Outdated
Comment thread python/ql/src/Security/CWE-285/PamAuthorization.ql Outdated
@@ -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. 😮

@@ -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 👍

@RasmusWL
RasmusWL requested a review from tausbn May 23, 2022 13:09
@tausbn
tausbn merged commit 3745526 into github:main May 23, 2022
@RasmusWL
RasmusWL deleted the promote-pam branch May 23, 2022 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants