Inline precededBySync

This commit is contained in:
Michael B. Gale
2023-01-05 16:18:30 +00:00
parent c252ec0414
commit abe38373da
2 changed files with 18 additions and 38 deletions

View File

@@ -90,13 +90,25 @@ predicate isWritableFileHandle(DataFlow::Node source, DataFlow::CallNode call) {
/**
* Holds if `os.File.Close` is called on `sink`.
*/
predicate isCloseSink(DataFlow::Node sink, DataFlow::CallNode call) {
predicate isCloseSink(DataFlow::Node sink, DataFlow::CallNode closeCall) {
// find calls to the os.File.Close function
call = any(CloseFileFun f).getACall() and
// that are deferred
unhandledCall(call) and
closeCall = any(CloseFileFun f).getACall() and
// that are unhandled
unhandledCall(closeCall) and
// where the function is called on the sink
call.getReceiver() = sink
closeCall.getReceiver() = sink and
// and check that it is not dominated by a call to `os.File.Sync`.
not exists(IR::Instruction syncInstr, DataFlow::Node syncReceiver, DataFlow::CallNode syncCall |
// match the instruction corresponding to an `os.File.Sync` call with the predecessor
syncCall.asInstruction() = syncInstr and
// check that the call to `os.File.Sync` is handled
isHandledSync(syncReceiver, syncCall) and
// find a predecessor to `closeCall` in the control flow graph which dominates the call to
// `os.File.Close`
syncInstr.dominatesNode(closeCall.asInstruction()) and
// check that `os.File.Sync` is called on the same object as `os.File.Close`
exists(DataFlow::SsaNode ssa | ssa.getAUse() = sink and ssa.getAUse() = syncReceiver)
)
}
/**
@@ -124,25 +136,6 @@ class UnhandledFileCloseDataFlowConfiguration extends DataFlow::Configuration {
override predicate isSink(DataFlow::Node sink) { isCloseSink(sink, _) }
}
/**
* Holds if a `node` is preceded by a call to `os.File.Sync`.
*/
predicate precededBySync(DataFlow::Node closeReceiver, DataFlow::CallNode closeCall) {
// using the control flow graph, try to find a call to a handled call to `os.File.Sync`
// which precedes `closeCall`.
exists(IR::Instruction syncInstr, DataFlow::Node syncReceiver, DataFlow::CallNode syncCall |
// match the instruction corresponding to an `os.File.Sync` call with the predecessor
syncCall.asInstruction() = syncInstr and
// check that the call to `os.File.Sync` is handled
isHandledSync(syncReceiver, syncCall) and
// find a predecessor to `closeCall` in the control flow graph which dominates the call to
// `os.File.Close`
syncInstr.dominatesNode(closeCall.asInstruction()) and
// check that `os.File.Sync` is called on the same object as `os.File.Close`
exists(DataFlow::SsaNode ssa | ssa.getAUse() = closeReceiver and ssa.getAUse() = syncReceiver)
)
}
from
UnhandledFileCloseDataFlowConfiguration cfg, DataFlow::PathNode source,
DataFlow::CallNode openCall, DataFlow::PathNode sink, DataFlow::CallNode closeCall
@@ -152,10 +145,7 @@ where
cfg.hasFlowPath(source, sink) and
isWritableFileHandle(source.getNode(), openCall) and
// get the `CallNode` corresponding to the sink
isCloseSink(sink.getNode(), closeCall) and
// check that the call to `os.File.Close` is not preceded by a checked call to
// `os.File.Sync`
not precededBySync(sink.getNode(), closeCall)
isCloseSink(sink.getNode(), closeCall)
select sink, source, sink,
"File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly.",
openCall, openCall.toString()

View File

@@ -2,8 +2,6 @@ edges
| tests.go:9:2:9:74 | ... := ...[0] : pointer type | tests.go:10:9:10:9 | f : pointer type |
| tests.go:10:9:10:9 | f : pointer type | tests.go:43:5:43:38 | ... := ...[0] : pointer type |
| tests.go:10:9:10:9 | f : pointer type | tests.go:60:5:60:38 | ... := ...[0] : pointer type |
| tests.go:10:9:10:9 | f : pointer type | tests.go:86:5:86:38 | ... := ...[0] : pointer type |
| tests.go:10:9:10:9 | f : pointer type | tests.go:96:5:96:38 | ... := ...[0] : pointer type |
| tests.go:10:9:10:9 | f : pointer type | tests.go:108:5:108:38 | ... := ...[0] : pointer type |
| tests.go:10:9:10:9 | f : pointer type | tests.go:124:5:124:38 | ... := ...[0] : pointer type |
| tests.go:18:2:18:69 | return statement[0] : pointer type | tests.go:53:5:53:42 | ... := ...[0] : pointer type |
@@ -21,9 +19,6 @@ edges
| tests.go:55:29:55:29 | f : pointer type | tests.go:25:32:25:32 | definition of f : pointer type |
| tests.go:60:5:60:38 | ... := ...[0] : pointer type | tests.go:62:3:62:3 | f |
| tests.go:70:5:70:42 | ... := ...[0] : pointer type | tests.go:72:3:72:3 | f |
| tests.go:86:5:86:38 | ... := ...[0] : pointer type | tests.go:89:4:89:4 | f |
| tests.go:86:5:86:38 | ... := ...[0] : pointer type | tests.go:91:3:91:3 | f |
| tests.go:96:5:96:38 | ... := ...[0] : pointer type | tests.go:99:9:99:9 | f |
| tests.go:108:5:108:38 | ... := ...[0] : pointer type | tests.go:110:9:110:9 | f |
| tests.go:124:5:124:38 | ... := ...[0] : pointer type | tests.go:128:3:128:3 | f |
nodes
@@ -45,11 +40,6 @@ nodes
| tests.go:62:3:62:3 | f | semmle.label | f |
| tests.go:70:5:70:42 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type |
| tests.go:72:3:72:3 | f | semmle.label | f |
| tests.go:86:5:86:38 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type |
| tests.go:89:4:89:4 | f | semmle.label | f |
| tests.go:91:3:91:3 | f | semmle.label | f |
| tests.go:96:5:96:38 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type |
| tests.go:99:9:99:9 | f | semmle.label | f |
| tests.go:108:5:108:38 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type |
| tests.go:110:9:110:9 | f | semmle.label | f |
| tests.go:124:5:124:38 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type |