From f811dff52744003e8ad5479b778be471eead6336 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Sat, 12 Sep 2020 12:21:07 +0200 Subject: [PATCH 1/3] Add taint-tracking for package `os` --- ql/src/semmle/go/frameworks/Stdlib.qll | 1 + ql/src/semmle/go/frameworks/stdlib/Os.qll | 82 +++++++++ .../go/frameworks/StdlibTaintFlow/Os.go | 164 ++++++++++++++++++ 3 files changed, 247 insertions(+) create mode 100644 ql/src/semmle/go/frameworks/stdlib/Os.qll create mode 100644 ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Os.go diff --git a/ql/src/semmle/go/frameworks/Stdlib.qll b/ql/src/semmle/go/frameworks/Stdlib.qll index b43886d6307..fd0c6d132af 100644 --- a/ql/src/semmle/go/frameworks/Stdlib.qll +++ b/ql/src/semmle/go/frameworks/Stdlib.qll @@ -29,6 +29,7 @@ import semmle.go.frameworks.stdlib.EncodingPem import semmle.go.frameworks.stdlib.EncodingXml import semmle.go.frameworks.stdlib.Html import semmle.go.frameworks.stdlib.HtmlTemplate +import semmle.go.frameworks.stdlib.Os import semmle.go.frameworks.stdlib.Path import semmle.go.frameworks.stdlib.PathFilepath import semmle.go.frameworks.stdlib.Reflect diff --git a/ql/src/semmle/go/frameworks/stdlib/Os.qll b/ql/src/semmle/go/frameworks/stdlib/Os.qll new file mode 100644 index 00000000000..4d9da3ad54a --- /dev/null +++ b/ql/src/semmle/go/frameworks/stdlib/Os.qll @@ -0,0 +1,82 @@ +/** + * Provides classes modeling security-relevant aspects of the `os` package. + */ + +import go + +/** Provides models of commonly used functions in the `os` package. */ +module Os { + private class FunctionModels extends TaintTracking::FunctionModel { + FunctionInput inp; + FunctionOutput outp; + + FunctionModels() { + // signature: func Expand(s string, mapping func(string) string) string + hasQualifiedName("os", "Expand") and + (inp.isParameter(0) and outp.isResult()) + or + // signature: func ExpandEnv(s string) string + hasQualifiedName("os", "ExpandEnv") and + (inp.isParameter(0) and outp.isResult()) + or + // signature: func NewFile(fd uintptr, name string) *File + hasQualifiedName("os", "NewFile") and + (inp.isParameter(0) and outp.isResult()) + or + // signature: func Pipe() (r *File, w *File, err error) + hasQualifiedName("os", "Pipe") and + (inp.isResult(1) and outp.isResult(0)) + } + + override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { + input = inp and output = outp + } + } + + private class MethodModels extends TaintTracking::FunctionModel, Method { + FunctionInput inp; + FunctionOutput outp; + + MethodModels() { + // signature: func (*File).Fd() uintptr + this.hasQualifiedName("os", "File", "Fd") and + (inp.isReceiver() and outp.isResult()) + or + // signature: func (*File).Read(b []byte) (n int, err error) + this.hasQualifiedName("os", "File", "Read") and + (inp.isReceiver() and outp.isParameter(0)) + or + // signature: func (*File).ReadAt(b []byte, off int64) (n int, err error) + this.hasQualifiedName("os", "File", "ReadAt") and + (inp.isReceiver() and outp.isParameter(0)) + or + // signature: func (*File).ReadFrom(r io.Reader) (n int64, err error) + this.hasQualifiedName("os", "File", "ReadFrom") and + (inp.isParameter(0) and outp.isReceiver()) + or + // signature: func (*File).SyscallConn() (syscall.RawConn, error) + this.hasQualifiedName("os", "File", "SyscallConn") and + ( + inp.isReceiver() and outp.isResult(0) + or + inp.isResult(0) and outp.isReceiver() + ) + or + // signature: func (*File).Write(b []byte) (n int, err error) + this.hasQualifiedName("os", "File", "Write") and + (inp.isParameter(0) and outp.isReceiver()) + or + // signature: func (*File).WriteAt(b []byte, off int64) (n int, err error) + this.hasQualifiedName("os", "File", "WriteAt") and + (inp.isParameter(0) and outp.isReceiver()) + or + // signature: func (*File).WriteString(s string) (n int, err error) + this.hasQualifiedName("os", "File", "WriteString") and + (inp.isParameter(0) and outp.isReceiver()) + } + + override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { + input = inp and output = outp + } + } +} diff --git a/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Os.go b/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Os.go new file mode 100644 index 00000000000..2215f41d119 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Os.go @@ -0,0 +1,164 @@ +// Code generated by https://github.com/gagliardetto/codebox. DO NOT EDIT. + +package main + +import ( + "io" + "os" + "syscall" +) + +func TaintStepTest_OsExpand_B0I0O0(sourceCQL interface{}) interface{} { + fromString656 := sourceCQL.(string) + intoString414 := os.Expand(fromString656, nil) + return intoString414 +} + +func TaintStepTest_OsExpandEnv_B0I0O0(sourceCQL interface{}) interface{} { + fromString518 := sourceCQL.(string) + intoString650 := os.ExpandEnv(fromString518) + return intoString650 +} + +func TaintStepTest_OsNewFile_B0I0O0(sourceCQL interface{}) interface{} { + fromUintptr784 := sourceCQL.(uintptr) + intoFile957 := os.NewFile(fromUintptr784, "") + return intoFile957 +} + +func TaintStepTest_OsPipe_B0I0O0(sourceCQL interface{}) interface{} { + fromFile520 := sourceCQL.(*os.File) + intoFile443, intermediateCQL, _ := os.Pipe() + link(fromFile520, intermediateCQL) + return intoFile443 +} + +func TaintStepTest_OsFileFd_B0I0O0(sourceCQL interface{}) interface{} { + fromFile127 := sourceCQL.(os.File) + intoUintptr483 := fromFile127.Fd() + return intoUintptr483 +} + +func TaintStepTest_OsFileRead_B0I0O0(sourceCQL interface{}) interface{} { + fromFile989 := sourceCQL.(os.File) + var intoByte982 []byte + fromFile989.Read(intoByte982) + return intoByte982 +} + +func TaintStepTest_OsFileReadAt_B0I0O0(sourceCQL interface{}) interface{} { + fromFile417 := sourceCQL.(os.File) + var intoByte584 []byte + fromFile417.ReadAt(intoByte584, 0) + return intoByte584 +} + +func TaintStepTest_OsFileReadFrom_B0I0O0(sourceCQL interface{}) interface{} { + fromReader991 := sourceCQL.(io.Reader) + var intoFile881 os.File + intoFile881.ReadFrom(fromReader991) + return intoFile881 +} + +func TaintStepTest_OsFileSyscallConn_B0I0O0(sourceCQL interface{}) interface{} { + fromFile186 := sourceCQL.(os.File) + intoRawConn284, _ := fromFile186.SyscallConn() + return intoRawConn284 +} + +func TaintStepTest_OsFileSyscallConn_B1I0O0(sourceCQL interface{}) interface{} { + fromRawConn908 := sourceCQL.(syscall.RawConn) + var intoFile137 os.File + intermediateCQL, _ := intoFile137.SyscallConn() + link(fromRawConn908, intermediateCQL) + return intoFile137 +} + +func TaintStepTest_OsFileWrite_B0I0O0(sourceCQL interface{}) interface{} { + fromByte494 := sourceCQL.([]byte) + var intoFile873 os.File + intoFile873.Write(fromByte494) + return intoFile873 +} + +func TaintStepTest_OsFileWriteAt_B0I0O0(sourceCQL interface{}) interface{} { + fromByte599 := sourceCQL.([]byte) + var intoFile409 os.File + intoFile409.WriteAt(fromByte599, 0) + return intoFile409 +} + +func TaintStepTest_OsFileWriteString_B0I0O0(sourceCQL interface{}) interface{} { + fromString246 := sourceCQL.(string) + var intoFile898 os.File + intoFile898.WriteString(fromString246) + return intoFile898 +} + +func RunAllTaints_Os() { + { + source := newSource(0) + out := TaintStepTest_OsExpand_B0I0O0(source) + sink(0, out) + } + { + source := newSource(1) + out := TaintStepTest_OsExpandEnv_B0I0O0(source) + sink(1, out) + } + { + source := newSource(2) + out := TaintStepTest_OsNewFile_B0I0O0(source) + sink(2, out) + } + { + source := newSource(3) + out := TaintStepTest_OsPipe_B0I0O0(source) + sink(3, out) + } + { + source := newSource(4) + out := TaintStepTest_OsFileFd_B0I0O0(source) + sink(4, out) + } + { + source := newSource(5) + out := TaintStepTest_OsFileRead_B0I0O0(source) + sink(5, out) + } + { + source := newSource(6) + out := TaintStepTest_OsFileReadAt_B0I0O0(source) + sink(6, out) + } + { + source := newSource(7) + out := TaintStepTest_OsFileReadFrom_B0I0O0(source) + sink(7, out) + } + { + source := newSource(8) + out := TaintStepTest_OsFileSyscallConn_B0I0O0(source) + sink(8, out) + } + { + source := newSource(9) + out := TaintStepTest_OsFileSyscallConn_B1I0O0(source) + sink(9, out) + } + { + source := newSource(10) + out := TaintStepTest_OsFileWrite_B0I0O0(source) + sink(10, out) + } + { + source := newSource(11) + out := TaintStepTest_OsFileWriteAt_B0I0O0(source) + sink(11, out) + } + { + source := newSource(12) + out := TaintStepTest_OsFileWriteString_B0I0O0(source) + sink(12, out) + } +} From 8eeb019b5caa090304cffa7c3e662e8cf8786928 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Sat, 12 Sep 2020 12:26:13 +0200 Subject: [PATCH 2/3] Move existing `OS` (all caps name) module classes to stdlib.Os module (notice the camelcase name) --- ql/src/semmle/go/frameworks/Stdlib.qll | 81 ----------------------- ql/src/semmle/go/frameworks/stdlib/Os.qll | 60 +++++++++++++++++ 2 files changed, 60 insertions(+), 81 deletions(-) diff --git a/ql/src/semmle/go/frameworks/Stdlib.qll b/ql/src/semmle/go/frameworks/Stdlib.qll index fd0c6d132af..f9dfbaad4ea 100644 --- a/ql/src/semmle/go/frameworks/Stdlib.qll +++ b/ql/src/semmle/go/frameworks/Stdlib.qll @@ -395,87 +395,6 @@ module IoUtil { } } -/** Provides models of commonly used functions in the `os` package. */ -module OS { - /** - * A call to a function in `os` that accesses the file system. - */ - private class OsFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode { - int pathidx; - - OsFileSystemAccess() { - exists(string fn | getTarget().hasQualifiedName("os", fn) | - fn = "Chdir" and pathidx = 0 - or - fn = "Chmod" and pathidx = 0 - or - fn = "Chown" and pathidx = 0 - or - fn = "Chtimes" and pathidx = 0 - or - fn = "Create" and pathidx = 0 - or - fn = "Lchown" and pathidx = 0 - or - fn = "Link" and pathidx in [0 .. 1] - or - fn = "Lstat" and pathidx = 0 - or - fn = "Mkdir" and pathidx = 0 - or - fn = "MkdirAll" and pathidx = 0 - or - fn = "NewFile" and pathidx = 1 - or - fn = "Open" and pathidx = 0 - or - fn = "OpenFile" and pathidx = 0 - or - fn = "Readlink" and pathidx = 0 - or - fn = "Remove" and pathidx = 0 - or - fn = "RemoveAll" and pathidx = 0 - or - fn = "Rename" and pathidx in [0 .. 1] - or - fn = "Stat" and pathidx = 0 - or - fn = "Symlink" and pathidx in [0 .. 1] - or - fn = "Truncate" and pathidx = 0 - ) - } - - override DataFlow::Node getAPathArgument() { result = getArgument(pathidx) } - } - - /** The `Expand` function. */ - class Expand extends TaintTracking::FunctionModel { - Expand() { hasQualifiedName("os", "Expand") } - - override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { - inp.isParameter(0) and outp.isResult() - } - } - - /** The `ExpandEnv` function. */ - class ExpandEnv extends TaintTracking::FunctionModel { - ExpandEnv() { hasQualifiedName("os", "ExpandEnv") } - - override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { - inp.isParameter(0) and outp.isResult() - } - } - - /** The `os.Exit` function, which ends the process. */ - private class Exit extends Function { - Exit() { hasQualifiedName("os", "Exit") } - - override predicate mayReturnNormally() { none() } - } -} - /** Provides a class for modeling functions which convert strings into integers. */ module IntegerParser { /** diff --git a/ql/src/semmle/go/frameworks/stdlib/Os.qll b/ql/src/semmle/go/frameworks/stdlib/Os.qll index 4d9da3ad54a..e5d8b91c148 100644 --- a/ql/src/semmle/go/frameworks/stdlib/Os.qll +++ b/ql/src/semmle/go/frameworks/stdlib/Os.qll @@ -6,6 +6,66 @@ import go /** Provides models of commonly used functions in the `os` package. */ module Os { + /** + * A call to a function in `os` that accesses the file system. + */ + private class OsFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode { + int pathidx; + + OsFileSystemAccess() { + exists(string fn | getTarget().hasQualifiedName("os", fn) | + fn = "Chdir" and pathidx = 0 + or + fn = "Chmod" and pathidx = 0 + or + fn = "Chown" and pathidx = 0 + or + fn = "Chtimes" and pathidx = 0 + or + fn = "Create" and pathidx = 0 + or + fn = "Lchown" and pathidx = 0 + or + fn = "Link" and pathidx in [0 .. 1] + or + fn = "Lstat" and pathidx = 0 + or + fn = "Mkdir" and pathidx = 0 + or + fn = "MkdirAll" and pathidx = 0 + or + fn = "NewFile" and pathidx = 1 + or + fn = "Open" and pathidx = 0 + or + fn = "OpenFile" and pathidx = 0 + or + fn = "Readlink" and pathidx = 0 + or + fn = "Remove" and pathidx = 0 + or + fn = "RemoveAll" and pathidx = 0 + or + fn = "Rename" and pathidx in [0 .. 1] + or + fn = "Stat" and pathidx = 0 + or + fn = "Symlink" and pathidx in [0 .. 1] + or + fn = "Truncate" and pathidx = 0 + ) + } + + override DataFlow::Node getAPathArgument() { result = getArgument(pathidx) } + } + + /** The `os.Exit` function, which ends the process. */ + private class Exit extends Function { + Exit() { hasQualifiedName("os", "Exit") } + + override predicate mayReturnNormally() { none() } + } + private class FunctionModels extends TaintTracking::FunctionModel { FunctionInput inp; FunctionOutput outp; From c89cfc886775b9b30d4c931d6eee8420a0c455e4 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Sun, 20 Sep 2020 14:33:40 +0200 Subject: [PATCH 3/3] Use go 1.14.3 --- ql/src/semmle/go/frameworks/stdlib/Os.qll | 4 -- .../go/frameworks/StdlibTaintFlow/Os.go | 63 ++++++++----------- 2 files changed, 25 insertions(+), 42 deletions(-) diff --git a/ql/src/semmle/go/frameworks/stdlib/Os.qll b/ql/src/semmle/go/frameworks/stdlib/Os.qll index e5d8b91c148..01f5e79a08d 100644 --- a/ql/src/semmle/go/frameworks/stdlib/Os.qll +++ b/ql/src/semmle/go/frameworks/stdlib/Os.qll @@ -110,10 +110,6 @@ module Os { this.hasQualifiedName("os", "File", "ReadAt") and (inp.isReceiver() and outp.isParameter(0)) or - // signature: func (*File).ReadFrom(r io.Reader) (n int64, err error) - this.hasQualifiedName("os", "File", "ReadFrom") and - (inp.isParameter(0) and outp.isReceiver()) - or // signature: func (*File).SyscallConn() (syscall.RawConn, error) this.hasQualifiedName("os", "File", "SyscallConn") and ( diff --git a/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Os.go b/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Os.go index 2215f41d119..221d6868cd5 100644 --- a/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Os.go +++ b/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Os.go @@ -3,7 +3,6 @@ package main import ( - "io" "os" "syscall" ) @@ -53,46 +52,39 @@ func TaintStepTest_OsFileReadAt_B0I0O0(sourceCQL interface{}) interface{} { return intoByte584 } -func TaintStepTest_OsFileReadFrom_B0I0O0(sourceCQL interface{}) interface{} { - fromReader991 := sourceCQL.(io.Reader) - var intoFile881 os.File - intoFile881.ReadFrom(fromReader991) - return intoFile881 -} - func TaintStepTest_OsFileSyscallConn_B0I0O0(sourceCQL interface{}) interface{} { - fromFile186 := sourceCQL.(os.File) - intoRawConn284, _ := fromFile186.SyscallConn() - return intoRawConn284 + fromFile991 := sourceCQL.(os.File) + intoRawConn881, _ := fromFile991.SyscallConn() + return intoRawConn881 } func TaintStepTest_OsFileSyscallConn_B1I0O0(sourceCQL interface{}) interface{} { - fromRawConn908 := sourceCQL.(syscall.RawConn) - var intoFile137 os.File - intermediateCQL, _ := intoFile137.SyscallConn() - link(fromRawConn908, intermediateCQL) - return intoFile137 + fromRawConn186 := sourceCQL.(syscall.RawConn) + var intoFile284 os.File + intermediateCQL, _ := intoFile284.SyscallConn() + link(fromRawConn186, intermediateCQL) + return intoFile284 } func TaintStepTest_OsFileWrite_B0I0O0(sourceCQL interface{}) interface{} { - fromByte494 := sourceCQL.([]byte) - var intoFile873 os.File - intoFile873.Write(fromByte494) - return intoFile873 + fromByte908 := sourceCQL.([]byte) + var intoFile137 os.File + intoFile137.Write(fromByte908) + return intoFile137 } func TaintStepTest_OsFileWriteAt_B0I0O0(sourceCQL interface{}) interface{} { - fromByte599 := sourceCQL.([]byte) - var intoFile409 os.File - intoFile409.WriteAt(fromByte599, 0) - return intoFile409 + fromByte494 := sourceCQL.([]byte) + var intoFile873 os.File + intoFile873.WriteAt(fromByte494, 0) + return intoFile873 } func TaintStepTest_OsFileWriteString_B0I0O0(sourceCQL interface{}) interface{} { - fromString246 := sourceCQL.(string) - var intoFile898 os.File - intoFile898.WriteString(fromString246) - return intoFile898 + fromString599 := sourceCQL.(string) + var intoFile409 os.File + intoFile409.WriteString(fromString599) + return intoFile409 } func RunAllTaints_Os() { @@ -133,32 +125,27 @@ func RunAllTaints_Os() { } { source := newSource(7) - out := TaintStepTest_OsFileReadFrom_B0I0O0(source) + out := TaintStepTest_OsFileSyscallConn_B0I0O0(source) sink(7, out) } { source := newSource(8) - out := TaintStepTest_OsFileSyscallConn_B0I0O0(source) + out := TaintStepTest_OsFileSyscallConn_B1I0O0(source) sink(8, out) } { source := newSource(9) - out := TaintStepTest_OsFileSyscallConn_B1I0O0(source) + out := TaintStepTest_OsFileWrite_B0I0O0(source) sink(9, out) } { source := newSource(10) - out := TaintStepTest_OsFileWrite_B0I0O0(source) + out := TaintStepTest_OsFileWriteAt_B0I0O0(source) sink(10, out) } { source := newSource(11) - out := TaintStepTest_OsFileWriteAt_B0I0O0(source) + out := TaintStepTest_OsFileWriteString_B0I0O0(source) sink(11, out) } - { - source := newSource(12) - out := TaintStepTest_OsFileWriteString_B0I0O0(source) - sink(12, out) - } }