hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-23 20:26:32 +01:00
Code Issues Packages Projects Releases Wiki Activity

Python: Modernise Statements/ queries

Browse Source 
Almost. Left out a few things marked with TODO
This commit is contained in:
Rasmus Wriedt Larsen
2020-02-12 13:01:05 +01:00
parent 83d40f167b
commit 13568b7b9f
22 changed files with 129 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
python/ql/src/Statements/DocStrings.ql
View File

@@ -27,8 +27,8 @@ predicate needs_docstring(Scope s) {
}
predicate function_needs_docstring(Function f) {
not exists(FunctionObject fo, FunctionObject base | fo.overrides(base) and fo.getFunction() = f |
not function_needs_docstring(base.getFunction())
not exists(FunctionValue fo, FunctionValue base | fo.overrides(base) and fo.getScope() = f |
not function_needs_docstring(base.getScope())
) and
f.getName() != "lambda" and
(f.getMetrics().getNumberOfLinesOfCode() - count(f.getADecorator())) > 2 and

20
python/ql/src/Statements/IterableStringOrSequence.ql
View File

@@ -13,21 +13,15 @@
import python
predicate is_a_string_type(ClassObject seqtype) {
seqtype = theBytesType() and major_version() = 2
or
seqtype = theUnicodeType()
}
from
For loop, ControlFlowNode iter, Object str, Object seq, ControlFlowNode seq_origin,
ClassObject strtype, ClassObject seqtype, ControlFlowNode str_origin
For loop, ControlFlowNode iter, Value str, Value seq, ControlFlowNode seq_origin, ControlFlowNode str_origin
where
loop.getIter().getAFlowNode() = iter and
iter.refersTo(str, strtype, str_origin) and
iter.refersTo(seq, seqtype, seq_origin) and
is_a_string_type(strtype) and
seqtype.isIterable() and
not is_a_string_type(seqtype)
select loop, "Iteration over $@, of class " + seqtype.getName() + ", may also iterate over $@.",
iter.pointsTo(str, str_origin) and
iter.pointsTo(seq, seq_origin) and
str.getClass() = ClassValue::str() and
seq.getClass().isIterable() and
not seq.getClass() = ClassValue::str()
select loop, "Iteration over $@, of class " + seq.getClass().getName() + ", may also iterate over $@.",
seq_origin, "sequence", str_origin, "string"

6
python/ql/src/Statements/MismatchInMultipleAssignment.ql
View File

@@ -36,15 +36,15 @@ predicate mismatched(Assign a, int lcount, int rcount, Location loc, string sequ
}
predicate mismatched_tuple_rhs(Assign a, int lcount, int rcount, Location loc) {
exists(ExprList l, TupleObject r, AstNode origin |
exists(ExprList l, TupleValue r, AstNode origin |
(
a.getATarget().(Tuple).getElts() = l or
a.getATarget().(List).getElts() = l
) and
a.getValue().refersTo(r, origin) and
a.getValue().pointsTo(r, origin) and
loc = origin.getLocation() and
lcount = len(l) and
rcount = r.getLength() and
rcount = r.length() and
lcount != rcount and
not exists(Starred s | l.getAnItem() = s)
)

14
python/ql/src/Statements/ModificationOfLocals.ql
View File

@@ -12,23 +12,17 @@
import python
Object aFunctionLocalsObject() {
exists(Call c, Name n, GlobalVariable v |
c = result.getOrigin() and
n = c.getFunc() and
n.getVariable() = v and
v.getId() = "locals" and
c.getScope() instanceof FastLocalsFunction
)
predicate originIsLocals(ControlFlowNode n) {
n.pointsTo(_, _, Value::named("locals").getACall())
}
predicate modification_of_locals(ControlFlowNode f) {
f.(SubscriptNode).getObject().refersTo(aFunctionLocalsObject()) and
originIsLocals(f.(SubscriptNode).getObject()) and
(f.isStore() or f.isDelete())
or
exists(string mname, AttrNode attr |
attr = f.(CallNode).getFunction() and
attr.getObject(mname).refersTo(aFunctionLocalsObject(), _)
originIsLocals(attr.getObject(mname))
|
mname = "pop" or
mname = "popitem" or

2
python/ql/src/Statements/NonIteratorInForLoop.qhelp
View File

@@ -17,7 +17,7 @@ for addressing the defect. </p>
</recommendation>
<example>
<p>
In this example, the loop may attempt to iterate over <code>None</code>, which is not an iterator.
In this example, the loop may attempt to iterate over <code>None</code>, which is not an iterable.
It is likely that the programmer forgot to test for <code>None</code> before the loop.
</p>
<sample src="NonIteratorInForLoop.py" />

2
python/ql/src/Statements/NonIteratorInForLoop.ql
View File

@@ -22,4 +22,4 @@ where
not t.failedInference(_) and
not v = Value::named("None") and
not t.isDescriptorType()
select loop, "$@ of class '$@' may be used in for-loop.", origin, "Non-iterator", t, t.getName()
select loop, "$@ of class '$@' may be used in for-loop.", origin, "Non-iterable", t, t.getName()

26
python/ql/src/Statements/RedundantAssignment.ql
View File

@@ -37,21 +37,30 @@ predicate maybe_defined_in_outer_scope(Name n) {
exists(SsaVariable v | v.getAUse().getNode() = n | v.maybeUndefined())
}
Variable relevant_var(Name n) {
n.getVariable() = result and
(corresponding(n, _) or corresponding(_, n))
/* Protection against FPs in projects that offer compatibility between Python 2 and 3,
* since many of them make assignments such as
*
* if PY2:
* bytes = str
* else:
* bytes = bytes
*
*/
predicate isBuiltin(string name) {
exists(Value v | v = Value::named(name) and v.isBuiltin())
}
predicate same_name(Name n1, Name n2) {
corresponding(n1, n2) and
relevant_var(n1) = relevant_var(n2) and
not exists(Object::builtin(n1.getId())) and
n1.getVariable() = n2.getVariable() and
not isBuiltin(n1.getId()) and
not maybe_defined_in_outer_scope(n2)
}
ClassObject value_type(Attribute a) { a.getObject().refersTo(_, result, _) }
predicate is_property_access(Attribute a) {
// TODO: We need something to model PropertyObject in the Value API
value_type(a).lookupAttribute(a.getName()) instanceof PropertyObject
}
@@ -77,9 +86,9 @@ predicate pyflakes_commented(AssignStmt assignment) {
}
predicate side_effecting_lhs(Attribute lhs) {
exists(ClassObject cls, ClassObject decl |
lhs.getObject().refersTo(_, cls, _) and
decl = cls.getAnImproperSuperType() and
exists(ClassValue cls, ClassValue decl |
lhs.getObject().pointsTo().getClass() = cls and
decl = cls.getASuperType() and
not decl.isBuiltin()
|
decl.declaresAttribute("__setattr__")
@@ -90,6 +99,7 @@ from AssignStmt a, Expr left, Expr right
where
assignment(a, left, right) and
same_value(left, right) and
// some people use self-assignment to shut Pyflakes up, such as `ok = ok # Pyflakes`
not pyflakes_commented(a) and
not side_effecting_lhs(left)
select a, "This assignment assigns a variable to itself."

8
python/ql/src/Statements/ShouldUseWithStatement.ql
View File

@@ -22,12 +22,12 @@ predicate only_stmt_in_finally(Try t, Call c) {
)
}
predicate points_to_context_manager(ControlFlowNode f, ClassObject cls) {
cls.isContextManager() and
forex(Object obj | f.refersTo(obj) | f.refersTo(obj, cls, _))
predicate points_to_context_manager(ControlFlowNode f, ClassValue cls) {
forex(Value v | f.pointsTo(v) | v.getClass() = cls) and
cls.isContextManager()
}
from Call close, Try t, ClassObject cls
from Call close, Try t, ClassValue cls
where
only_stmt_in_finally(t, close) and
calls_close(close) and

40
python/ql/src/Statements/StatementNoEffect.ql
View File

@@ -13,28 +13,28 @@
import python
predicate understood_attribute(Attribute attr, ClassObject cls, ClassObject attr_cls) {
predicate understood_attribute(Attribute attr, ClassValue cls, ClassValue attr_cls) {
exists(string name | attr.getName() = name |
attr.getObject().refersTo(_, cls, _) and
cls.attributeRefersTo(name, _, attr_cls, _)
attr.getObject().pointsTo().getClass() = cls and
cls.attr(name).getClass() = attr_cls
)
}
/* Conservative estimate of whether attribute lookup has a side effect */
predicate side_effecting_attribute(Attribute attr) {
exists(ClassObject cls, ClassObject attr_cls |
exists(ClassValue cls, ClassValue attr_cls |
understood_attribute(attr, cls, attr_cls) and
side_effecting_descriptor_type(attr_cls)
)
}
predicate maybe_side_effecting_attribute(Attribute attr) {
not understood_attribute(attr, _, _) and not attr.refersTo(_)
not understood_attribute(attr, _, _) and not attr.pointsTo(_)
or
side_effecting_attribute(attr)
}
predicate side_effecting_descriptor_type(ClassObject descriptor) {
predicate side_effecting_descriptor_type(ClassValue descriptor) {
descriptor.isDescriptorType() and
/*
* Technically all descriptor gets have side effects,
@@ -42,9 +42,9 @@ predicate side_effecting_descriptor_type(ClassObject descriptor) {
* we want to treat them as having no effect.
*/
not descriptor = thePyFunctionType() and
not descriptor = theStaticMethodType() and
not descriptor = theClassMethodType()
not descriptor = ClassValue::function() and
not descriptor = ClassValue::staticmethod() and
not descriptor = ClassValue::classmethod()
}
/**
@@ -52,39 +52,39 @@ predicate side_effecting_descriptor_type(ClassObject descriptor) {
* side-effecting unless we know otherwise.
*/
predicate side_effecting_binary(Expr b) {
exists(Expr sub, ClassObject cls, string method_name |
exists(Expr sub, ClassValue cls, string method_name |
binary_operator_special_method(b, sub, cls, method_name)
or
comparison_special_method(b, sub, cls, method_name)
|
method_name = special_method() and
cls.hasAttribute(method_name) and
not exists(ClassObject declaring |
not exists(ClassValue declaring |
declaring.declaresAttribute(method_name) and
declaring = cls.getAnImproperSuperType() and
declaring = cls.getASuperType() and
declaring.isBuiltin() and
not declaring = theObjectType()
not declaring = ClassValue::object()
)
)
}
pragma[nomagic]
private predicate binary_operator_special_method(
BinaryExpr b, Expr sub, ClassObject cls, string method_name
BinaryExpr b, Expr sub, ClassValue cls, string method_name
) {
method_name = special_method() and
sub = b.getLeft() and
method_name = b.getOp().getSpecialMethodName() and
sub.refersTo(_, cls, _)
sub.pointsTo().getClass() = cls
}
pragma[nomagic]
private predicate comparison_special_method(Compare b, Expr sub, ClassObject cls, string method_name) {
private predicate comparison_special_method(Compare b, Expr sub, ClassValue cls, string method_name) {
exists(Cmpop op |
b.compares(sub, op, _) and
method_name = op.getSpecialMethodName()
) and
sub.refersTo(_, cls, _)
sub.pointsTo().getClass() = cls
}
private string special_method() {
@@ -102,9 +102,8 @@ predicate is_notebook(File f) {
/** Expression (statement) in a jupyter/ipython notebook */
predicate in_notebook(Expr e) { is_notebook(e.getScope().(Module).getFile()) }
FunctionObject assertRaises() {
result =
ModuleObject::named("unittest").attr("TestCase").(ClassObject).lookupAttribute("assertRaises")
FunctionValue assertRaises() {
result = Value::named("unittest.TestCase").(ClassValue).lookup("assertRaises")
}
/** Holds if expression `e` is in a `with` block that tests for exceptions being raised. */
@@ -124,6 +123,7 @@ predicate python2_print(Expr e) {
}
predicate no_effect(Expr e) {
// strings can be used as comments
not e instanceof StrConst and
not e.hasSideEffects() and
forall(Expr sub | sub = e.getASubExpression*() |

2
python/ql/src/Statements/StringConcatenationInLoop.qhelp
View File

@@ -9,7 +9,7 @@ of iterations.</p>
</overview>
<recommendation>
<p>Initialize an empty list before the start of the list.
<p>Initialize an empty list before the start of the loop.
During the loop append the substrings to the list.
At the end of the loop, convert the list to a string by using <code>''.join(list)</code>.</p>

5
python/ql/src/Statements/StringConcatenationInLoop.ql
View File

@@ -14,13 +14,12 @@ import python
predicate string_concat_in_loop(BinaryExpr b) {
b.getOp() instanceof Add and
exists(SsaVariable d, SsaVariable u, BinaryExprNode add, ClassObject str_type |
exists(SsaVariable d, SsaVariable u, BinaryExprNode add |
add.getNode() = b and d = u.getAnUltimateDefinition()
|
d.getDefinition().(DefinitionNode).getValue() = add and
u.getAUse() = add.getAnOperand() and
add.getAnOperand().refersTo(_, str_type, _) and
(str_type = theBytesType() or str_type = theUnicodeType())
add.getAnOperand().pointsTo().getClass() = ClassValue::str()
)
}

1
python/ql/src/Statements/TopLevelPrint.ql
View File

@@ -34,6 +34,7 @@ predicate is_print_stmt(Stmt s) {
from Stmt p
where
is_print_stmt(p) and
// TODO: Need to discuss how we would like to handle ModuleObject.getKind in the glorious future
exists(ModuleObject m | m.getModule() = p.getScope() and m.getKind() = "module") and
not exists(If i | main_eq_name(i) and i.getASubStatement().getASubStatement*() = p)
select p, "Print statement may execute during import."

4
python/ql/src/Statements/UnnecessaryDelete.ql
View File

@@ -27,8 +27,8 @@ where
* reference cycle, and an explicit call to `del` helps break this cycle.
*/
not exists(FunctionObject ex |
ex.hasLongName("sys.exc_info") and
not exists(FunctionValue ex |
ex = Value::named("sys.exc_info") and
ex.getACall().getScope() = f
)
select del, "Unnecessary deletion of local variable $@ in function $@.", e.getLocation(),

6
python/ql/src/Statements/UnusedExceptionObject.ql
View File

@@ -12,9 +12,9 @@
import python
from Call call, ClassObject ex
from Call call, ClassValue ex
where
call.getFunc().refersTo(ex) and
ex.getAnImproperSuperType() = theExceptionType() and
call.getFunc().pointsTo(ex) and
ex.getASuperType() = ClassValue::exception() and
exists(ExprStmt s | s.getValue() = call)
select call, "Instantiating an exception, but not raising it, has no effect"

2
python/ql/src/Statements/UseOfExit.ql
View File

@@ -12,7 +12,7 @@
import python
from CallNode call, string name
where call.getFunction().refersTo(Object::quitter(name))
where call.getFunction().pointsTo(Value::siteQuitter(name))
select call,
"The '" + name +
"' site.Quitter object may not exist if the 'site' module is not loaded or is modified."

19
python/ql/src/semmle/python/objects/ObjectAPI.qll
View File

@@ -301,6 +301,19 @@ module Value {
result = ObjectInternal::none_()
}
/**
* Shorcuts added by the `site` module to exit your interactive session.
*
* see https://docs.python.org/3/library/constants.html#constants-added-by-the-site-module
*/
Value siteQuitter(string name) {
(
name = "exit"
or
name = "quit"
) and
result = Value::named(name)
}
}
/** Class representing callables in the Python program
@@ -418,6 +431,12 @@ class ClassValue extends Value {
this.hasAttribute("__get__")
}
/** Holds if this class is a context manager. */
predicate isContextManager() {
this.hasAttribute("__enter__") and
this.hasAttribute("__exit__")
}
/** Gets the qualified name for this class.
* Should return the same name as the `__qualname__` attribute on classes in Python 3.
*/

6
python/ql/test/query-tests/Statements/general/MismatchInMultipleAssignment.expected
View File

@@ -1,4 +1,4 @@
| statements_test.py:19:5:19:18 | AssignStmt | Left hand side of assignment contains 3 variables, but right hand side is a $@ of length 2. | statements_test.py:19:15:19:18 | statements_test.py:19 | tuple |
| statements_test.py:167:5:167:23 | AssignStmt | Left hand side of assignment contains 3 variables, but right hand side is a $@ of length 5. | statements_test.py:167:13:167:23 | statements_test.py:167 | list |
| statements_test.py:176:5:176:48 | AssignStmt | Left hand side of assignment contains 3 variables, but right hand side is a $@ of length 5. | statements_test.py:171:16:171:24 | statements_test.py:171 | tuple |
| statements_test.py:176:5:176:48 | AssignStmt | Left hand side of assignment contains 3 variables, but right hand side is a $@ of length 6. | statements_test.py:173:16:173:26 | statements_test.py:173 | tuple |
| statements_test.py:169:5:169:23 | AssignStmt | Left hand side of assignment contains 3 variables, but right hand side is a $@ of length 5. | statements_test.py:169:13:169:23 | statements_test.py:169 | list |
| statements_test.py:178:5:178:48 | AssignStmt | Left hand side of assignment contains 3 variables, but right hand side is a $@ of length 5. | statements_test.py:173:16:173:24 | statements_test.py:173 | tuple |
| statements_test.py:178:5:178:48 | AssignStmt | Left hand side of assignment contains 3 variables, but right hand side is a $@ of length 6. | statements_test.py:175:16:175:26 | statements_test.py:175 | tuple |

2
python/ql/test/query-tests/Statements/general/NonIteratorInForLoop.expected
View File

@@ -1 +1 @@
| test.py:50:1:50:23 | For | $@ of class '$@' may be used in for-loop. | test.py:50:10:50:22 | ControlFlowNode for NonIterator() | Non-iterator | test.py:45:1:45:26 | class NonIterator | NonIterator |
| test.py:50:1:50:23 | For | $@ of class '$@' may be used in for-loop. | test.py:50:10:50:22 | ControlFlowNode for NonIterator() | Non-iterable | test.py:45:1:45:26 | class NonIterator | NonIterator |

2
python/ql/test/query-tests/Statements/general/RedundantAssignment.expected
View File

@@ -1,3 +1,3 @@
| statements_test.py:54:5:54:9 | AssignStmt | This assignment assigns a variable to itself. |
| statements_test.py:57:9:57:19 | AssignStmt | This assignment assigns a variable to itself. |
| statements_test.py:117:9:117:23 | AssignStmt | This assignment assigns a variable to itself. |
| statements_test.py:119:9:119:23 | AssignStmt | This assignment assigns a variable to itself. |

2
python/ql/test/query-tests/Statements/general/UnnecessaryDelete.expected
View File

@@ -1 +1 @@
| statements_test.py:185:5:185:9 | Delete | Unnecessary deletion of local variable $@ in function $@. | statements_test.py:185:9:185:9 | statements_test.py:185 | x | statements_test.py:183:1:183:31 | statements_test.py:183 | error_unnecessary_delete |
| statements_test.py:187:5:187:9 | Delete | Unnecessary deletion of local variable $@ in function $@. | statements_test.py:187:9:187:9 | statements_test.py:187 | x | statements_test.py:185:1:185:31 | statements_test.py:185 | error_unnecessary_delete |

4
python/ql/test/query-tests/Statements/general/UnnecessaryElseClause.expected
View File

@@ -1,2 +1,2 @@
| statements_test.py:63:1:63:19 | For | This 'for' statement has a redundant 'else' as no 'break' is present in the body. |
| statements_test.py:68:1:68:13 | While | This 'while' statement has a redundant 'else' as no 'break' is present in the body. |
| statements_test.py:65:1:65:19 | For | This 'for' statement has a redundant 'else' as no 'break' is present in the body. |
| statements_test.py:70:1:70:13 | While | This 'while' statement has a redundant 'else' as no 'break' is present in the body. |

6
python/ql/test/query-tests/Statements/general/statements_test.py
View File

@@ -56,8 +56,10 @@ class Redundant(object):
def __init__(self, args):
args = args # violation
#Non redundant assignment
len = len
if sys.version_info < (3,):
bytes = str
else:
bytes = bytes # Should not be flagged
#Pointless else clauses
for x in range(10):