Merge branch 'main' into henrymercer/check-query-ids

This commit is contained in:
Henry Mercer
2022-12-08 13:05:46 +00:00
committed by GitHub
682 changed files with 14110 additions and 4191 deletions

View File

@@ -1,3 +1,7 @@
## 0.6.5
No user-facing changes.
## 0.6.4
### Minor Analysis Improvements

View File

@@ -0,0 +1,4 @@
---
category: majorAnalysis
---
* The _PAM authorization bypass due to incorrect usage_ (`py/pam-auth-bypass`) query has been converted to a taint-tracking query, resulting in significantly fewer false positives.

View File

@@ -0,0 +1,3 @@
## 0.6.5
No user-facing changes.

View File

@@ -1,2 +1,2 @@
---
lastReleaseVersion: 0.6.4
lastReleaseVersion: 0.6.5

View File

@@ -1,5 +1,5 @@
name: codeql/python-all
version: 0.6.5-dev
version: 0.6.6-dev
groups: python
dbscheme: semmlecode.python.dbscheme
extractor: python

View File

@@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.RemoteFlowSources
private import semmle.python.dataflow.new.TaintTracking
private import semmle.python.Frameworks
private import semmle.python.security.internal.EncryptionKeySizes
/**
* A data-flow node that executes an operating system command,
@@ -1141,21 +1142,21 @@ module Cryptography {
abstract class RsaRange extends Range {
final override string getName() { result = "RSA" }
final override int minimumSecureKeySize() { result = 2048 }
final override int minimumSecureKeySize() { result = minSecureKeySizeRsa() }
}
/** A data-flow node that generates a new DSA key-pair. */
abstract class DsaRange extends Range {
final override string getName() { result = "DSA" }
final override int minimumSecureKeySize() { result = 2048 }
final override int minimumSecureKeySize() { result = minSecureKeySizeDsa() }
}
/** A data-flow node that generates a new ECC key-pair. */
abstract class EccRange extends Range {
final override string getName() { result = "ECC" }
final override int minimumSecureKeySize() { result = 224 }
final override int minimumSecureKeySize() { result = minSecureKeySizeEcc() }
}
}
}

View File

@@ -244,4 +244,20 @@ module Consistency {
not callable = viableCallable(call) and
not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
}
query predicate uniqueParameterNodeAtPosition(
DataFlowCallable c, ParameterPosition pos, Node p, string msg
) {
isParameterNode(p, c, pos) and
not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and
msg = "Parameters with overlapping positions."
}
query predicate uniqueParameterNodePosition(
DataFlowCallable c, ParameterPosition pos, Node p, string msg
) {
isParameterNode(p, c, pos) and
not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and
msg = "Parameter node with multiple positions."
}
}

View File

@@ -0,0 +1,61 @@
/**
* Provides default sources, sinks and sanitizers for detecting
* "PAM Authorization" vulnerabilities.
*/
import python
import semmle.python.ApiGraphs
import semmle.python.dataflow.new.TaintTracking
import semmle.python.dataflow.new.RemoteFlowSources
/**
* Provides default sources, sinks and sanitizers for detecting
* "PAM Authorization" vulnerabilities.
*/
module PamAuthorizationCustomizations {
/**
* Models a node corresponding to the `pam` library
*/
API::Node libPam() {
exists(API::CallNode findLibCall, API::CallNode cdllCall |
findLibCall =
API::moduleImport("ctypes").getMember("util").getMember("find_library").getACall() and
findLibCall.getParameter(0).getAValueReachingSink().asExpr().(StrConst).getText() = "pam" and
cdllCall = API::moduleImport("ctypes").getMember("CDLL").getACall() and
cdllCall.getParameter(0).getAValueReachingSink() = findLibCall
|
result = cdllCall.getReturn()
)
}
/**
* A data flow source for "PAM Authorization" vulnerabilities.
*/
abstract class Source extends DataFlow::Node { }
/**
* A data flow sink for "PAM Authorization" vulnerabilities.
*/
abstract class Sink extends DataFlow::Node { }
/**
* A source of remote user input, considered as a flow source.
*/
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
/**
* A vulnerable `pam_authenticate` call considered as a flow sink.
*/
class VulnPamAuthCall extends API::CallNode, Sink {
VulnPamAuthCall() {
exists(DataFlow::Node h |
this = libPam().getMember("pam_authenticate").getACall() and
h = this.getArg(0) and
not exists(API::CallNode acctMgmtCall |
acctMgmtCall = libPam().getMember("pam_acct_mgmt").getACall() and
DataFlow::localFlow(h, acctMgmtCall.getArg(0))
)
)
}
}
}

View File

@@ -0,0 +1,39 @@
/**
* Provides a taint-tracking configuration for detecting "PAM Authorization" vulnerabilities.
*
* Note, for performance reasons: only import this file if
* `PamAuthorization::Configuration` is needed, otherwise
* `PamAuthorizationCustomizations` should be imported instead.
*/
import python
import semmle.python.ApiGraphs
import semmle.python.dataflow.new.TaintTracking
import PamAuthorizationCustomizations::PamAuthorizationCustomizations
/**
* A taint-tracking configuration for detecting "PAM Authorization" vulnerabilities.
*/
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "PamAuthorization" }
override predicate isSource(DataFlow::Node node) { node instanceof Source }
override predicate isSink(DataFlow::Node node) { node instanceof Sink }
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
// Models flow from a remotely supplied username field to a PAM `handle`.
// `retval = pam_start(service, username, byref(conv), byref(handle))`
exists(API::CallNode pamStart, DataFlow::Node handle, API::CallNode pointer |
pointer = API::moduleImport("ctypes").getMember(["pointer", "byref"]).getACall() and
pamStart = libPam().getMember("pam_start").getACall() and
pointer = pamStart.getArg(3) and
handle = pointer.getArg(0) and
pamStart.getArg(1) = node1 and
handle = node2
)
or
// Flow from handle to the authenticate call in the final step
exists(VulnPamAuthCall c | c.getArg(0) = node1 | node2 = c)
}
}

View File

@@ -0,0 +1,21 @@
/**
* INTERNAL: Do not use.
*
* Provides predicates for recommended encryption key sizes.
* Such that we can share this logic across our CodeQL analysis of different languages.
*/
/** Returns the minimum recommended key size for RSA. */
int minSecureKeySizeRsa() { result = 2048 }
/** Returns the minimum recommended key size for DSA. */
int minSecureKeySizeDsa() { result = 2048 }
/** Returns the minimum recommended key size for DH. */
int minSecureKeySizeDh() { result = 2048 }
/** Returns the minimum recommended key size for elliptic curve cryptography. */
int minSecureKeySizeEcc() { result = 256 }
/** Returns the minimum recommended key size for AES. */
int minSecureKeySizeAes() { result = 128 }

View File

@@ -1,3 +1,7 @@
## 0.5.5
No user-facing changes.
## 0.5.4
No user-facing changes.

View File

@@ -1,7 +1,7 @@
/**
* @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
* @kind path-problem
* @problem.severity warning
* @security-severity 8.1
* @precision high
@@ -11,28 +11,12 @@
*/
import python
import DataFlow::PathGraph
import semmle.python.ApiGraphs
import experimental.semmle.python.Concepts
import semmle.python.dataflow.new.TaintTracking
import semmle.python.security.dataflow.PamAuthorizationQuery
API::Node libPam() {
exists(API::CallNode findLibCall, API::CallNode cdllCall |
findLibCall = API::moduleImport("ctypes").getMember("util").getMember("find_library").getACall() and
findLibCall.getParameter(0).getAValueReachingSink().asExpr().(StrConst).getText() = "pam" and
cdllCall = API::moduleImport("ctypes").getMember("CDLL").getACall() and
cdllCall.getParameter(0).getAValueReachingSink() = findLibCall
|
result = cdllCall.getReturn()
)
}
from API::CallNode authenticateCall, DataFlow::Node handle
where
authenticateCall = libPam().getMember("pam_authenticate").getACall() and
handle = authenticateCall.getArg(0) and
not exists(API::CallNode acctMgmtCall |
acctMgmtCall = libPam().getMember("pam_acct_mgmt").getACall() and
DataFlow::localFlow(handle, acctMgmtCall.getArg(0))
)
select authenticateCall,
"This PAM authentication call may lead to an authorization bypass, since 'pam_acct_mgmt' is not called afterwards."
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
where config.hasFlowPath(source, sink)
select sink.getNode(), source, sink,
"This PAM authentication depends on a $@, and 'pam_acct_mgmt' is not called afterwards.",
source.getNode(), "user-provided value"

View File

@@ -11,13 +11,13 @@ As computational power increases, the ability to break ciphers grows and keys ne
<p>
The three main asymmetric key algorithms currently in use are RivestShamirAdleman (RSA) cryptography, Digital Signature Algorithm (DSA), and Elliptic-curve cryptography (ECC).
With current technology, key sizes of 2048 bits for RSA and DSA,
or 224 bits for ECC, are regarded as unbreakable.
or 256 bits for ECC, are regarded as unbreakable.
</p>
</overview>
<recommendation>
<p>
Increase the key size to the recommended amount or larger. For RSA or DSA this is at least 2048 bits, for ECC this is at least 224 bits.
Increase the key size to the recommended amount or larger. For RSA or DSA this is at least 2048 bits, for ECC this is at least 256 bits.
</p>
</recommendation>
@@ -45,4 +45,3 @@ Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Len
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Bumped the minimum keysize we consider secure for elliptic curve cryptography from 224 to 256 bits, following current best practices. This might effect results from the _Use of weak cryptographic key_ (`py/weak-crypto-key`) query.

View File

@@ -0,0 +1,3 @@
## 0.5.5
No user-facing changes.

View File

@@ -1,2 +1,2 @@
---
lastReleaseVersion: 0.5.4
lastReleaseVersion: 0.5.5

View File

@@ -1,5 +1,5 @@
name: codeql/python-queries
version: 0.5.5-dev
version: 0.5.6-dev
groups:
- python
- queries

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -19,3 +19,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -67,3 +67,5 @@ reverseRead
argHasPostUpdate
postWithInFlow
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition

View File

@@ -1 +1,16 @@
| 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. |
edges
| pam_test.py:0:0:0:0 | ModuleVariableNode for pam_test.request | pam_test.py:71:16:71:22 | ControlFlowNode for request |
| pam_test.py:4:26:4:32 | ControlFlowNode for ImportMember | pam_test.py:4:26:4:32 | GSSA Variable request |
| pam_test.py:4:26:4:32 | GSSA Variable request | pam_test.py:0:0:0:0 | ModuleVariableNode for pam_test.request |
| pam_test.py:71:16:71:22 | ControlFlowNode for request | pam_test.py:71:16:71:27 | ControlFlowNode for Attribute |
| pam_test.py:71:16:71:27 | ControlFlowNode for Attribute | pam_test.py:76:14:76:40 | ControlFlowNode for pam_authenticate() |
nodes
| pam_test.py:0:0:0:0 | ModuleVariableNode for pam_test.request | semmle.label | ModuleVariableNode for pam_test.request |
| pam_test.py:4:26:4:32 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
| pam_test.py:4:26:4:32 | GSSA Variable request | semmle.label | GSSA Variable request |
| pam_test.py:71:16:71:22 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
| pam_test.py:71:16:71:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| pam_test.py:76:14:76:40 | ControlFlowNode for pam_authenticate() | semmle.label | ControlFlowNode for pam_authenticate() |
subpaths
#select
| pam_test.py:76:14:76:40 | ControlFlowNode for pam_authenticate() | pam_test.py:4:26:4:32 | ControlFlowNode for ImportMember | pam_test.py:76:14:76:40 | ControlFlowNode for pam_authenticate() | This PAM authentication depends on a $@, and 'pam_acct_mgmt' is not called afterwards. | pam_test.py:4:26:4:32 | ControlFlowNode for ImportMember | user-provided value |

View File

@@ -1,6 +1,7 @@
from ctypes import CDLL, POINTER, Structure, byref
from ctypes import c_char_p, c_int
from ctypes.util import find_library
from flask import Flask, request, redirect
class PamHandle(Structure):
@@ -38,26 +39,57 @@ pam_acct_mgmt.restype = c_int
pam_acct_mgmt.argtypes = [PamHandle, c_int]
class pam():
def authenticate_bad_but_no_alert(self, username, service='login'):
# This is not OK, but since we don't have flow from a remote-flow-source, we
# don't give an alert.
handle = PamHandle()
conv = PamConv(None, 0)
retval = pam_start(service, username, byref(conv), byref(handle))
retval = pam_authenticate(handle, 0)
# NOT OK: no call to `pam_acct_mgmt`
auth_success = retval == 0
def authenticate_bad(self, username, service='login'):
handle = PamHandle()
conv = PamConv(None, 0)
retval = pam_start(service, username, byref(conv), byref(handle))
return auth_success
retval = pam_authenticate(handle, 0)
auth_success = retval == 0
return auth_success
def authenticate_good(self, username, service='login'):
handle = PamHandle()
conv = PamConv(None, 0)
retval = pam_start(service, username, byref(conv), byref(handle))
def authenticate_good(self, username, service='login'):
handle = PamHandle()
conv = PamConv(None, 0)
retval = pam_start(service, username, byref(conv), byref(handle))
retval = pam_authenticate(handle, 0)
if retval == 0:
retval = pam_acct_mgmt(handle, 0)
auth_success = retval == 0
retval = pam_authenticate(handle, 0)
if retval == 0:
retval = pam_acct_mgmt(handle, 0)
auth_success = retval == 0
return auth_success
return auth_success
app = Flask(__name__)
@app.route('/bad')
def bad():
username = request.args.get('username', '')
handle = PamHandle()
conv = PamConv(None, 0)
retval = pam_start(service, username, byref(conv), byref(handle))
retval = pam_authenticate(handle, 0)
# NOT OK: no call to `pam_acct_mgmt`
auth_success = retval == 0
return auth_success
@app.route('/good')
def good():
username = request.args.get('username', '')
handle = PamHandle()
conv = PamConv(None, 0)
retval = pam_start(service, username, byref(conv), byref(handle))
retval = pam_authenticate(handle, 0)
if retval == 0:
retval = pam_acct_mgmt(handle, 0)
auth_success = retval == 0
return auth_success

View File

@@ -1,8 +1,8 @@
| weak_crypto.py:68:1:68:21 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 |
| weak_crypto.py:69:1:69:19 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 163 |
| weak_crypto.py:69:1:69:19 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 256 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 224 |
| weak_crypto.py:70:1:70:28 | ControlFlowNode for rsa_gen_key() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |
| weak_crypto.py:72:1:72:30 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 |
| weak_crypto.py:73:1:73:25 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 163 |
| weak_crypto.py:73:1:73:25 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 256 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 224 |
| weak_crypto.py:74:1:74:37 | ControlFlowNode for rsa_gen_key() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |
| weak_crypto.py:76:1:76:22 | ControlFlowNode for Attribute() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 |
| weak_crypto.py:77:1:77:22 | ControlFlowNode for Attribute() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |

View File

@@ -19,8 +19,8 @@ DSA_STRONG = 3076
BIG = 10000
EC_WEAK = ec.SECT163K1() # has key size of 163
EC_OK = ec.SECP224R1()
EC_WEAK = ec.SECP224R1()
EC_OK = ec.SECP256R1()
EC_STRONG = ec.SECP384R1()
EC_BIG = ec.SECT571R1()