-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Expand file tree
/
Copy pathMissingErrorCheck.ql
More file actions
131 lines (122 loc) · 5.03 KB
/
MissingErrorCheck.ql
File metadata and controls
131 lines (122 loc) · 5.03 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
/**
* @name Missing error check
* @description When a function returns a pointer alongside an error value, one should normally
* assume that the pointer may be nil until either the pointer or error has been checked.
* @kind problem
* @problem.severity warning
* @id go/missing-error-check
* @tags quality
* reliability
* error-handling
* external/cwe/cwe-252
* @precision high
*/
import go
/**
* Holds if `node` is a reference to the `nil` builtin constant.
*/
predicate isNil(DataFlow::Node node) { node = Builtin::nil().getARead() }
/**
* Matches if `call` may return a nil pointer alongside an error value.
*
* This is both an over- and under-estimate: over in that we assume opaque functions may use this
* convention, and under in that functions with bodies are only recognized if they use a literal
* `nil` for the pointer return value at some return site.
*/
predicate calleeMayReturnNilWithError(DataFlow::CallNode call) {
not exists(call.getACallee())
or
exists(FuncDef callee | callee = call.getACallee() |
not exists(callee.getBody())
or
exists(IR::ReturnInstruction ret, DataFlow::Node ptrReturn, DataFlow::Node errReturn |
callee = ret.getRoot() and
ptrReturn = DataFlow::instructionNode(ret.getResult(0)) and
errReturn = DataFlow::instructionNode(ret.getResult(1)) and
isNil(ptrReturn) and
not isNil(errReturn)
)
)
}
/**
* Matches if `type` is a pointer, slice or interface type, or an alias for such a type.
*/
predicate isDereferenceableType(Type maybePointer) {
exists(Type t | t = maybePointer.getUnderlyingType() |
t instanceof PointerType or t instanceof SliceType or t instanceof InterfaceType
)
}
/**
* Matches if `instruction` checks `value`.
*
* We consider testing value for equality (against anything), passing it as a parameter to
* a function call, switching on either its value or its type or casting it to constitute a
* check.
*/
predicate checksValue(IR::Instruction instruction, DataFlow::SsaNode value) {
exists(DataFlow::InstructionNode instNode | instNode.asInstruction() = instruction |
instNode.(DataFlow::CallNode).getAnArgument() = value.getAUse() or
instNode.(DataFlow::EqualityTestNode).getAnOperand() = value.getAUse()
)
or
value.getAUse().asInstruction() = instruction and
(
exists(ExpressionSwitchStmt s | instruction.(IR::EvalInstruction).getExpr() = s.getExpr())
or
// This case accounts for both a type-switch or cast used to check `value`
exists(TypeAssertExpr e | instruction.(IR::EvalInstruction).getExpr() = e.getExpr())
)
}
// Now that we have use-use flow, phi nodes aren't directly involved in the flow graph. TODO: change this?
DataFlow::SsaNode phiDefinedFrom(DataFlow::SsaNode node) {
result.getDefinition().(SsaPseudoDefinition).getAnInput() = node.getDefinition().getVariable()
}
DataFlow::SsaNode definedFrom(DataFlow::SsaNode node) {
DataFlow::localFlow(node, result) or
result = phiDefinedFrom*(node)
}
/**
* Matches if `call` is a function returning (`ptr`, `err`) where `ptr` may be nil, and neither
* `ptr` not `err` has been checked for validity as of `node`.
*
* This is initially true of any callsite that may call either an opaque function or a user-defined
* function that may return (nil, error), and is true of any downstream control-flow node where a
* check has not certainly been made against either `ptr` or `err`.
*/
predicate returnUncheckedAtNode(
DataFlow::CallNode call, ControlFlow::Node node, DataFlow::SsaNode ptr, DataFlow::SsaNode err
) {
(
// Base case: check that `ptr` and `err` have appropriate types, and that the callee may return
// a nil pointer with an error.
ptr.getAPredecessor() = call.getResult(0) and
err.getAPredecessor() = call.getResult(1) and
call.asInstruction() = node and
isDereferenceableType(ptr.getType()) and
err.getType() instanceof ErrorType and
calleeMayReturnNilWithError(call)
or
// Recursive case: check that some predecessor is missing a check, and `node` does not itself
// check either `ptr` or `err`.
// localFlow is used to permit checks via either an SSA phi node or ordinary assignment.
returnUncheckedAtNode(call, node.getAPredecessor(), ptr, err) and
not exists(DataFlow::SsaNode checked |
checked = definedFrom(ptr) or checked = definedFrom(err)
|
checksValue(node, checked)
)
)
}
from
DataFlow::CallNode call, DataFlow::SsaNode ptr, DataFlow::SsaNode err,
DataFlow::PointerDereferenceNode deref, ControlFlow::Node derefNode
where
// `derefNode` is a control-flow node corresponding to `deref`
deref.getOperand().asInstruction() = derefNode and
// neither `ptr` nor `err`, the return values of `call`, have been checked as of `derefNode`
returnUncheckedAtNode(call, derefNode, ptr, err) and
// `deref` dereferences `ptr`
deref.getOperand() = ptr.getAUse()
select deref.getOperand(),
"$@ may be nil at this dereference because $@ may not have been checked.", ptr,
ptr.getSourceVariable().toString(), err, err.getSourceVariable().toString()