hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-01-29 22:32:58 +01:00
Code Issues Packages Projects Releases Wiki Activity

Teach SsaWithFields to properly handle implicit dereferences.

Browse Source
This commit is contained in:
Max Schaefer
2020-04-03 18:05:17 +01:00
parent 407493094d
commit 76f2748cbc
5 changed files with 44 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
change-notes/1.24/analysis-go.md
View File

@@ -26,6 +26,7 @@ The CodeQL library for Go now contains a folder of simple "cookbook" queries tha
| **Query** | **Expected impact** | **Change** |
|-------------------------------------------------------------------------------|-----------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| Arbitrary file write during zip extraction ("zip slip") (`go/zipslip`) | Fewer false positives | The query now recognizes more cases where it is safe to use a path extractor from an archive. |
| Bitwise exclusive-or used like exponentiation (`go/mistyped-exponentiation`) | Fewer false positives | The query now identifies when the value of an xor is assigned to a mask object, and excludes such results. |
| Command built from user-controlled sources (`go/command-injection`) | More results | The library models used by the query have been improved, allowing it to flag more potentially problematic cases, including sources that flow into shells, sudo, or programming-language interpreters as arguments. |
| Database query built from user-controlled sources (`go/sql-injection`) | More results | The library models used by the query have been improved, allowing it to flag more potentially problematic cases. |

8
ql/src/semmle/go/controlflow/IR.qll
View File

@@ -1555,4 +1555,12 @@ module IR {
EvalImplicitMaxSliceBoundInstruction implicitMaxSliceBoundInstruction(SliceExpr e) {
result = MkImplicitMaxSliceBound(e)
}
/**
* Gets the implicit dereference instruction for `e`, where `e` is a pointer used as the base
* in a field/method access, element access, or slice expression.
*/
EvalImplicitDerefInstruction implicitDerefInstruction(Expr e) {
result = MkImplicitDeref(e)
}
}

10
ql/src/semmle/go/dataflow/SSA.qll
View File

@@ -311,9 +311,13 @@ private TSsaWithFields accessPath(IR::Instruction insn) {
* by ssa-with-fields value `base`.
*/
private IR::Instruction accessPathAux(TSsaWithFields base, Field f) {
exists(IR::FieldReadInstruction fr | fr = result |
base = accessPath(fr.getBase()) and
f = fr.getField()
exists(IR::FieldReadInstruction fr, IR::EvalInstruction frb |
fr.getBase() = frb or
fr.getBase() = IR::implicitDerefInstruction(frb.getExpr())
|
base = accessPath(frb) and
f = fr.getField() and
result = fr
)
}

6
ql/test/query-tests/Security/CWE-022/ZipSlip.expected
View File

@@ -1,13 +1,15 @@
edges
| ZipSlip.go:12:24:12:29 | selection of Name : string | ZipSlip.go:14:20:14:20 | p |
| tarslip.go:14:23:14:33 | selection of Name : string | tarslip.go:14:14:14:34 | call to Dir |
| tst.go:15:11:15:16 | selection of Name : string | tst.go:20:20:20:23 | path |
nodes
| ZipSlip.go:12:24:12:29 | selection of Name : string | semmle.label | selection of Name : string |
| ZipSlip.go:14:20:14:20 | p | semmle.label | p |
| tarslip.go:14:14:14:34 | call to Dir | semmle.label | call to Dir |
| tarslip.go:14:23:14:33 | selection of Name : string | semmle.label | selection of Name : string |
| tst.go:15:11:15:16 | selection of Name : string | semmle.label | selection of Name : string |
| tst.go:20:20:20:23 | path | semmle.label | path |
| tst.go:32:21:32:26 | selection of Name | semmle.label | selection of Name |
#select
| ZipSlip.go:12:24:12:29 | selection of Name | ZipSlip.go:12:24:12:29 | selection of Name : string | ZipSlip.go:14:20:14:20 | p | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.go:14:20:14:20 | p | file system operation |
| tarslip.go:14:23:14:33 | selection of Name | tarslip.go:14:23:14:33 | selection of Name : string | tarslip.go:14:14:14:34 | call to Dir | Unsanitized archive entry, which may contain '..', is used in a $@. | tarslip.go:14:14:14:34 | call to Dir | file system operation |
| tst.go:15:11:15:16 | selection of Name | tst.go:15:11:15:16 | selection of Name : string | tst.go:20:20:20:23 | path | Unsanitized archive entry, which may contain '..', is used in a $@. | tst.go:20:20:20:23 | path | file system operation |
| tst.go:32:21:32:26 | selection of Name | tst.go:32:21:32:26 | selection of Name | tst.go:32:21:32:26 | selection of Name | Unsanitized archive entry, which may contain '..', is used in a $@. | tst.go:32:21:32:26 | selection of Name | file system operation |

24
ql/test/query-tests/Security/CWE-022/tarslip.go Normal file
View File

@@ -0,0 +1,24 @@
package main
import (
"archive/tar"
"io"
"os"
"path"
"strings"
)
func untarBad(reader io.Reader, prefix string) {
tarReader := tar.NewReader(reader)
header, _ := tarReader.Next()
os.MkdirAll(path.Dir(header.Name), 0755) // NOT OK
}
func untarGood(reader io.Reader, prefix string) {
tarReader := tar.NewReader(reader)
header, _ := tarReader.Next()
if !strings.HasPrefix(header.Name, prefix) {
panic("tar contents corrupted")
}
os.MkdirAll(path.Dir(header.Name), 0755) // OK
}