This repository was archived by the owner on Jan 5, 2023. It is now read-only.
Golang : Add Query To Detect PAM Authorization Bugs#709
Conversation
Contributor
|
Is there an assosciated bounty app? |
2 tasks
Using merely `pam_authneticate` call to grant access to a user can cause security issue. The `pam_authenticate` call only checks if the username and the password match. It does not check if the account is expired. Hence, a user with an expired login or an expired password can still login This issue is fairly common and was recently found in [gogs/gogs](https://www.github.com/gogs/gogs) and [go-gitea/gitea](https://www.github.com/go-gitea/gitea). In this case, the bugs were assigned [CVE-2022-0871](https://nvd.nist.gov/vuln/detail/CVE-2022-0871) and [CVE-2022-0905](https://nvd.nist.gov/vuln/detail/CVE-2022-0905) respectiely. This PR aims to detect instances were an initiated PAM Transaction invokes the `Authenticate` method but does not invoke a call to the `AcctMgmt` method. Due to the nature of the query, I don't exect that may FP's or FN's. There could some unwanted alerts due to test code but to avoid that I filter out all file paths which have `test` in their name. With this PR, I submit two queries, one using local data flow and the other using global taint flow. The global taint flow one should be more precise but may have a performance penalty while the other local flow one may lead to FN's but should be faster to execute. A quick run of both these queries on roughly 17 projects can be found in the LGTM runs below. * [global taint flow](https://lgtm.com/query/8603256578345367962/) * [local data flow](https://lgtm.com/query/5088026685932206344/) All of the results appear to be correct. The runs also includes two projects `procupineyhairs/gogs_pam` and `porcupineyhairs/gitea_pam`. These are vulnerable versions of the projects mentioned above. The alerts show a valid detection of the CVE. No alerts for the `gogs` and `gitea` projects shows detection of the patch correcting the bug. I am also working on two more PR's for C and Python respectively which detect the same underlying issue. minor
This was referenced May 24, 2022
emersion
pushed a commit
to emersion/webpass
that referenced
this pull request
May 24, 2022
Not using `pam_acct_mgmt` after `pam_authenticate` to check the validity of a login can lead to an authorization bypass ## Common Weakness Enumeration(CWE) category CWE - 863 ## Root Cause Analysis In this case, in the following PAM transaction, only a call to `pam.Authenticate` is used to login a user. https://lgtm.com/projects/g/emersion/webpass/snapshot/c3217efc9100880b960b1e085c30d5f95c3d3471/files/config/auth_pam.go#x72bc4d6069cdb224:1 This implies that a user with expired credentials can still login. The bug can be verified easily by creating a new user account, expiring it with <code>chage -E0 `username` </code> and then trying to log in with the expired credentials. ## Remediation This can be fixed by invoking a call to `pam.AcctMgmt` after a successful call to `pam.Authenticate` ## Common Vulnerability Scoring System Vector ### Exploitability The attack can be carried over the network. A complex non-standard configuration or a specialized condition is required for the attack to be successfully conducted. The attacker also requires access to a users credentials, be it expired, for an attack to be successful. There is no user interaction required for successful execution. The attack can affect components outside the scope of the target module. ### Impact Using this attack vector, an attacker may access otherwise restricted parts of the system. The attack can be used to gain access to confidential files like passwords, login credentials and other secrets. Hence, it has a high impact on confidentiality. It may also be directly used to affect a change on a system resource. Hence has a medium to high impact on integrity. This attack may not be used to affect the availability of the system. Taking this account an appropriate CVSS v3.1 vector would be (AV:N/AC:H/PR:L/UI:N/S:C/C:H/I:L/A:N)[https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:N/A:L&version=3.1] This gives it a base score of 7.7/10 and a severity rating of high. ## References * [Man Page for pam_acct_mgmt](https://man7.org/linux/man-pages/man3/pam_acct_mgmt.3.html) * [CWE-863](http://cwe.mitre.org/data/definitions/863.html) * [CWE-285](http://cwe.mitre.org/data/definitions/285.html) * github/securitylab#561 * github/codeql-go#709 ### This bug was found using *[CodeQL by Github](https://codeql.github.com/)*
This was referenced May 24, 2022
matzf
pushed a commit
to netsec-ethz/scion-apps
that referenced
this pull request
May 27, 2022
…229) Not using `pam_acct_mgmt` after `pam_authenticate` to check the validity of a login can lead to an authorization bypass ## Common Weakness Enumeration(CWE) category CWE - 863 ## Root Cause Analysis In this case, in the following PAM transaction, only a call to `pam.Authenticate` is used to login a user. https://lgtm.com/projects/g/netsec-ethz/scion-apps/snapshot/52ffbbafc507ba067d56eb9ec482b41c19fc84ed/files/ssh/server/ssh/authcallbacks.go#xecf1a4150da8e10d:1 This implies that a user with expired credentials can still login. The bug can be verified easily by creating a new user account, expiring it with <code>chage -E0 `username` </code> and then trying to log in with the expired credentials. ## Remediation This can be fixed by invoking a call to `pam.AcctMgmt` after a successful call to `pam.Authenticate` ## Common Vulnerability Scoring System Vector ### Exploitability The attack can be carried over the network. A complex non-standard configuration or a specialized condition is required for the attack to be successfully conducted. The attacker also requires access to a users credentials, be it expired, for an attack to be successful. There is no user interaction required for successful execution. The attack can affect components outside the scope of the target module. ### Impact Using this attack vector, an attacker may access otherwise restricted parts of the system. The attack can be used to gain access to confidential files like passwords, login credentials and other secrets. Hence, it has a high impact on confidentiality. It may also be directly used to affect a change on a system resource. Hence has a medium to high impact on integrity. This attack may not be used to affect the availability of the system. Taking this account an appropriate CVSS v3.1 vector would be (AV:N/AC:H/PR:L/UI:N/S:C/C:H/I:L/A:N)[https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:N/A:L&version=3.1] This gives it a base score of 7.7/10 and a severity rating of high. ## References * [Man Page for pam_acct_mgmt](https://man7.org/linux/man-pages/man3/pam_acct_mgmt.3.html) * [CWE-863](http://cwe.mitre.org/data/definitions/863.html) * [CWE-285](http://cwe.mitre.org/data/definitions/285.html) * github/securitylab#561 * github/codeql-go#709 ### This bug was found using *[CodeQL by Github](https://codeql.github.com/)*
edospadoni
pushed a commit
to nethesis/nethvoice-report
that referenced
this pull request
May 27, 2022
…ity of a login can lead to an authorization bypass (#172) CWE - 863 In this case, in the following PAM transaction, only a call to `pam.Authenticate` is used to login a user. https://lgtm.com/projects/g/nethesis/nethvoice-report/snapshot/4c37ee73a69564895b9db44ec2959766f0fa7bfe/files/api/methods/auth.go#x4b20c1ca54cad81d:1 This implies that a user with expired credentials can still login. The bug can be verified easily by creating a new user account, expiring it with <code>chage -E0 `username` </code> and then trying to log in with the expired credentials. This can be fixed by invoking a call to `pam.AcctMgmt` after a successful call to `pam.Authenticate` The attack can be carried over the network. A complex non-standard configuration or a specialized condition is required for the attack to be successfully conducted. The attacker also requires access to a users credentials, be it expired, for an attack to be successful. There is no user interaction required for successful execution. The attack can affect components outside the scope of the target module. Using this attack vector, an attacker may access otherwise restricted parts of the system. The attack can be used to gain access to confidential files like passwords, login credentials and other secrets. Hence, it has a high impact on confidentiality. It may also be directly used to affect a change on a system resource. Hence has a medium to high impact on integrity. This attack may not be used to affect the availability of the system. Taking this account an appropriate CVSS v3.1 vector would be (AV:N/AC:H/PR:L/UI:N/S:C/C:H/I:L/A:N)[https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:N/A:L&version=3.1] This gives it a base score of 7.7/10 and a severity rating of high. * [Man Page for pam_acct_mgmt](https://man7.org/linux/man-pages/man3/pam_acct_mgmt.3.html) * [CWE-863](http://cwe.mitre.org/data/definitions/863.html) * [CWE-285](http://cwe.mitre.org/data/definitions/285.html) * github/securitylab#561 * github/codeql-go#709
smowton
reviewed
May 30, 2022
Contributor
smowton
left a comment
There was a problem hiding this comment.
Please also rebase this PR against https://github.com/github/codeql/tree/main/go as the codeql-go repo has been retired.
| @@ -0,0 +1,49 @@ | |||
| /** | |||
| * @name PAM Authentication Bypass | |||
Contributor
There was a problem hiding this comment.
What is the local version for? It should be strictly a subset of the global one, right?
Author
There was a problem hiding this comment.
The global one has a longer runtime but is more accurate. The local one has a shorter runtime but would miss some edge cases; Not that I have found any in the wild.
Author
|
@smowton I have created a PR with the requested changes. I am closing this one now. Superseded by github/codeql#9377 |
This pull request was closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Using merely
pam_authneticatecall to grant access to a user can cause security issue. Thepam_authenticatecall only checks if the username and the password match. It does not check if the account is expired. Hence, a user with an expired login or an expired password can still login.This issue is fairly common and was recently found in gogs/gogs and go-gitea/gitea. In this case, the bugs were assigned CVE-2022-0871 and CVE-2022-0905 respectively.
This PR aims to detect instances were an initiated PAM Transaction invokes the
Authenticatemethod but does not invoke a call to theAcctMgmtmethod. Due to the nature of the query, I don't expect that may FP's or FN's. There could some unwanted alerts due to test code but to avoid that I filter out all file paths which havetestin them.With this PR, I submit two queries, one using local data flow and the other using global taint flow. The global taint flow one should be more precise but may have a performance penalty while the other local flow one may lead to FN's but should be faster to execute.
A quick run of both these queries on roughly 17 projects can be found in the LGTM runs below.
All of the results appear to be correct. The runs also includes two projects
procupineyhairs/gogs_pamandporcupineyhairs/gitea_pam. These are vulnerable versions of the projects mentioned above. The alerts show a valid detection of the CVE. No alerts for thegogsandgiteaprojects shows detection of the patch correcting the bug.I am also working on two more PR's for C and Python respectively which detect the same underlying issue.