Skip to content

Commit b7f874d

Browse files
am0o0am0o0
committed
fix tests, better afero support!
1 parent c5faddc commit b7f874d

File tree

10 files changed

+1255
-137
lines changed

10 files changed

+1255
-137
lines changed

‎go/ql/lib/semmle/go/security/FileSystemAccess.qll

Lines changed: 88 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import go
22

3-
43
/**
54
* The File system access sinks of `net/http` package
65
*/
@@ -127,55 +126,100 @@ class FiberSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
127126
override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
128127
}
129128

130-
string aferoPackage() { result = "github.com/spf13/afero" }
131-
132129
/**
133-
* Provide File system access sinks of [afero](https://github.com/spf13/afero) filesystem framework
130+
* Provide File system access sinks of [afero](https://github.com/spf13/afero) framework
134131
* The Types that are not vulnerable: `afero.BasePathFs` and `afero.IOFS`
135132
*/
136-
class AferoSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
137-
int pathArg;
133+
module Afero {
134+
string aferoPackage() { result = "github.com/spf13/afero" }
135+
136+
/**
137+
* Provide File system access sinks of [afero](https://github.com/spf13/afero) framework methods
138+
*/
139+
class AferoSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
140+
int pathArg;
141+
142+
AferoSystemAccess() {
143+
exists(Method f |
144+
f.hasQualifiedName(package(aferoPackage(), ""), "HttpFs",
145+
["Create", "Open", "OpenFile", "Remove", "RemoveAll"]) and
146+
this = f.getACall() and
147+
pathArg = 0
148+
or
149+
f.hasQualifiedName(package(aferoPackage(), ""), "RegexpFs",
150+
["Create", "Open", "OpenFile", "Remove", "RemoveAll", "Mkdir", "MkdirAll"]) and
151+
this = f.getACall() and
152+
pathArg = 0
153+
or
154+
f.hasQualifiedName(package(aferoPackage(), ""), "ReadOnlyFs",
155+
["Create", "Open", "OpenFile", "ReadDir", "ReadlinkIfPossible", "Mkdir", "MkdirAll"]) and
156+
this = f.getACall() and
157+
pathArg = 0
158+
or
159+
f.hasQualifiedName(package(aferoPackage(), ""), "OsFs",
160+
[
161+
"Create", "Open", "OpenFile", "ReadlinkIfPossible", "Remove", "RemoveAll", "Mkdir",
162+
"MkdirAll"
163+
]) and
164+
this = f.getACall() and
165+
pathArg = 0
166+
or
167+
f.hasQualifiedName(package(aferoPackage(), ""), "MemMapFs",
168+
["Create", "Open", "OpenFile", "Remove", "RemoveAll", "Mkdir", "MkdirAll"]) and
169+
this = f.getACall() and
170+
pathArg = 0
171+
)
172+
}
173+
174+
override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
175+
}
176+
177+
/**
178+
* Provide File system access sinks of [afero](https://github.com/spf13/afero) framework utility functions
179+
* The Types that are not vulnerable: `afero.BasePathFs` and `afero.IOFS`
180+
*/
181+
class AferoUtilityFunctionSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
182+
int pathArg;
183+
184+
AferoUtilityFunctionSystemAccess() {
185+
// utility functions
186+
exists(Function f |
187+
f.hasQualifiedName(package(aferoPackage(), ""),
188+
["WriteReader", "SafeWriteReader", "WriteFile", "ReadFile", "ReadDir"]) and
189+
this = f.getACall() and
190+
pathArg = 1 and
191+
not aferoSanitizer(this.getArgument(0))
192+
)
193+
}
194+
195+
override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
196+
}
138197

139-
AferoSystemAccess() {
140-
// utility functions
198+
/**
199+
* A sanitizer for when the Afero utility functions has a first argument of a safe type like NewBasePathFs
200+
*
201+
* e.g.
202+
* ```
203+
* basePathFs := afero.NewBasePathFs(osFS, "tmp")
204+
* afero.ReadFile(basePathFs, filepath)
205+
* ```
206+
*/
207+
predicate aferoSanitizer(DataFlow::Node n) {
141208
exists(Function f |
142-
f.hasQualifiedName(package(aferoPackage(), ""),
143-
["WriteReader", "SafeWriteReader", "WriteFile", "ReadFile", "ReadDir"]) and
144-
this = f.getACall() and
145-
pathArg = 1
146-
)
147-
or
148-
// afero FS Types
149-
exists(Method f |
150-
f.hasQualifiedName(package(aferoPackage(), ""), "HttpFs",
151-
["Create", "Open", "OpenFile", "Remove", "RemoveAll"]) and
152-
this = f.getACall() and
153-
pathArg = 0
154-
or
155-
f.hasQualifiedName(package(aferoPackage(), ""), "RegexpFs",
156-
["Create", "Open", "OpenFile", "Remove", "RemoveAll", "Mkdir", "MkdirAll"]) and
157-
this = f.getACall() and
158-
pathArg = 0
159-
or
160-
f.hasQualifiedName(package(aferoPackage(), ""), "ReadOnlyFs",
161-
["Create", "Open", "OpenFile", "ReadDir", "ReadlinkIfPossible", "Mkdir", "MkdirAll"]) and
162-
this = f.getACall() and
163-
pathArg = 0
164-
or
165-
f.hasQualifiedName(package(aferoPackage(), ""), "OsFs",
166-
[
167-
"Create", "Open", "OpenFile", "ReadlinkIfPossible", "Remove", "RemoveAll", "Mkdir",
168-
"MkdirAll"
169-
]) and
170-
this = f.getACall() and
171-
pathArg = 0
172-
or
173-
f.hasQualifiedName(package(aferoPackage(), ""), "MemMapFs",
174-
["Create", "Open", "OpenFile", "Remove", "RemoveAll", "Mkdir", "MkdirAll"]) and
175-
this = f.getACall() and
176-
pathArg = 0
209+
f.hasQualifiedName(package(aferoPackage(), ""), "NewBasePathFs") and
210+
TaintTracking::localTaint(f.getACall(), n)
177211
)
178212
}
179213

180-
override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
214+
/**
215+
* A helper for `aferoSanitizer` for when the Afero instance is initialized with one of the safe FS types like IOFS
216+
*
217+
* e.g.`n2 := &afero.Afero{Fs: afero.NewBasePathFs(osFS, "./")}` n1 is `afero.NewBasePathFs(osFS, "./")`
218+
*/
219+
predicate additionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
220+
exists(StructLit st | st.getType().hasQualifiedName(package(aferoPackage(), ""), "Afero") |
221+
n1.asExpr() = st.getAChildExpr*() and
222+
n2.asExpr() = st
223+
)
224+
}
181225
}

‎go/ql/test/library-tests/semmle/go/security/FileSystemAccess/FileSystemAccess.expected

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,28 @@
1-
| main.go:26:6:26:69 | call to SaveToFileWithBuffer |
2-
| main.go:33:14:33:76 | call to WriteFile |
3-
| main.go:34:16:34:45 | call to ReadFile |
4-
| main.go:41:14:41:49 | call to ReadFile |
5-
| main.go:47:14:47:47 | call to ReadFile |
6-
| main.go:52:14:52:49 | call to ReadFile |
7-
| main.go:57:14:57:52 | call to ReadFile |
8-
| main.go:70:14:70:35 | call to ReadFile |
9-
| main.go:87:10:87:41 | call to File |
10-
| main.go:91:10:91:72 | call to Attachment |
11-
| main.go:100:10:100:29 | call to SendFile |
12-
| main.go:105:7:105:34 | call to SaveFile |
13-
| main.go:106:3:106:24 | call to Attachment |
14-
| main.go:107:10:107:29 | call to SendFile |
15-
| main.go:116:7:116:45 | call to SendFile |
16-
| main.go:117:3:117:32 | call to SendFile |
17-
| main.go:118:3:118:29 | call to SendFileBytes |
18-
| main.go:120:7:120:62 | call to SaveMultipartFile |
19-
| main.go:121:3:121:43 | call to ServeFile |
20-
| main.go:122:3:122:55 | call to ServeFileUncompressed |
21-
| main.go:123:3:123:40 | call to ServeFileBytes |
22-
| main.go:124:3:124:52 | call to ServeFileBytesUncompressed |
23-
| main.go:152:3:152:18 | call to File |
24-
| main.go:153:3:153:47 | call to ServeFile |
25-
| main.go:154:3:154:53 | call to FileAttachment |
26-
| main.go:156:7:156:40 | call to SaveUploadedFile |
1+
| main.go:23:2:23:48 | call to Download |
2+
| main.go:25:6:25:85 | call to SaveToFileWithBuffer |
3+
| main.go:33:14:33:41 | call to MkdirAll |
4+
| main.go:34:14:34:75 | call to WriteFile |
5+
| main.go:35:14:35:77 | call to WriteFile |
6+
| main.go:36:14:36:75 | call to WriteFile |
7+
| main.go:37:16:37:45 | call to ReadFile |
8+
| main.go:39:14:39:32 | call to Open |
9+
| main.go:41:14:41:61 | call to SafeWriteReader |
10+
| main.go:42:14:42:57 | call to WriteReader |
11+
| main.go:48:14:48:47 | call to ReadFile |
12+
| main.go:53:14:53:49 | call to ReadFile |
13+
| main.go:58:14:58:52 | call to ReadFile |
14+
| main.go:63:17:63:37 | call to Open |
15+
| main.go:94:10:94:41 | call to File |
16+
| main.go:98:10:98:72 | call to Attachment |
17+
| main.go:108:7:108:34 | call to SaveFile |
18+
| main.go:109:10:109:29 | call to SendFile |
19+
| main.go:116:6:116:35 | call to SendFile |
20+
| main.go:117:6:117:49 | call to SendFileWithRate |
21+
| main.go:118:6:118:28 | call to ServeFile |
22+
| main.go:119:6:119:42 | call to ServeFileWithRate |
23+
| main.go:120:12:120:45 | call to UploadFormFiles |
24+
| main.go:122:9:122:46 | call to SaveFormFile |
25+
| main.go:129:3:129:18 | call to File |
26+
| main.go:130:3:130:47 | call to ServeFile |
27+
| main.go:131:3:131:53 | call to FileAttachment |
28+
| main.go:133:7:133:40 | call to SaveUploadedFile |

‎go/ql/test/library-tests/semmle/go/security/FileSystemAccess/main.go

Lines changed: 43 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -3,82 +3,89 @@ package main
33
import (
44
"fmt"
55
beego "github.com/beego/beego/v2/server/web"
6+
BeegoContext "github.com/beego/beego/v2/server/web/context"
67
"github.com/gin-gonic/gin"
78
"github.com/gofiber/fiber/v2"
8-
"github.com/kataras/iris/v12"
9+
"github.com/kataras/iris/v12/context"
910
"github.com/labstack/echo/v4"
1011
"github.com/spf13/afero"
11-
"github.com/valyala/fasthttp"
12-
"mime/multipart"
13-
"net"
1412
"net/http"
13+
"os"
1514
"regexp"
1615
)
1716

18-
type MainController struct {
19-
beego.Controller
17+
func main() {
18+
return
2019
}
2120

22-
func (c *MainController) Get() {
23-
filepath := c.Ctx.Request.URL.Query()["filepath"][0]
24-
c.Ctx.Output.Download(filepath, "license.txt")
21+
func BeegoController(beegoController beego.Controller) {
22+
beegoOutput := BeegoContext.BeegoOutput{}
23+
beegoOutput.Download("filepath", "license.txt")
2524
buffer := make([]byte, 10)
26-
_ = c.SaveToFileWithBuffer("filenameExistsInForm", filepath, buffer)
27-
25+
_ = beegoController.SaveToFileWithBuffer("filenameExistsInForm", "filepath", buffer)
2826
}
2927

3028
func Afero(writer http.ResponseWriter, request *http.Request) {
3129
filepath := request.URL.Query()["filepath"][0]
30+
//osFS := afero.NewMemMapFs()
31+
// OR
3232
osFS := afero.NewOsFs()
33-
fmt.Println(afero.WriteFile(osFS, filepath, []byte("this is me d !"), 0755))
33+
fmt.Println(osFS.MkdirAll("tmp/b", 0755))
34+
fmt.Println(afero.WriteFile(osFS, "tmp/a", []byte("this is me a !"), 0755))
35+
fmt.Println(afero.WriteFile(osFS, "tmp/b/c", []byte("this is me c !"), 0755))
36+
fmt.Println(afero.WriteFile(osFS, "tmp/d", []byte("this is me d !"), 0755))
3437
content, _ := afero.ReadFile(osFS, filepath)
3538
fmt.Println(string(content))
3639
fmt.Println(osFS.Open(filepath))
40+
// BAD
41+
fmt.Println(afero.SafeWriteReader(osFS, filepath, os.Stdout))
42+
fmt.Println(afero.WriteReader(osFS, filepath, os.Stdout))
3743

38-
// BasePathFs
39-
fmt.Println("BasePathFs:")
40-
basePathFs := afero.NewBasePathFs(osFS, "tmp")
41-
fmt.Println(afero.ReadFile(basePathFs, filepath))
42-
43-
// RegexpFs
44+
// RegexpFs ==> BAD
4445
fmt.Println("RegexpFs:")
4546
regex, _ := regexp.Compile(".*")
4647
regexpFs := afero.NewRegexpFs(osFS, regex)
4748
fmt.Println(afero.ReadFile(regexpFs, filepath))
4849

49-
// ReadOnlyFS
50+
// ReadOnlyFS ==> BAD
5051
fmt.Println("ReadOnlyFS:")
5152
readOnlyFS := afero.NewReadOnlyFs(osFS)
5253
fmt.Println(afero.ReadFile(readOnlyFS, filepath))
5354

54-
// CacheOnReadFs
55+
// CacheOnReadFs ==> BAD
5556
fmt.Println("CacheOnReadFs:")
5657
cacheOnReadFs := afero.NewCacheOnReadFs(osFS, osFS, 10)
5758
fmt.Println(afero.ReadFile(cacheOnReadFs, filepath))
5859

59-
// HttpFS
60+
// HttpFS ==> BAD
6061
fmt.Println("HttpFS:")
6162
httpFs := afero.NewHttpFs(osFS)
6263
httpFile, _ := httpFs.Open(filepath)
6364
tmpbytes := make([]byte, 30)
6465
fmt.Println(httpFile.Read(tmpbytes))
6566
fmt.Println(string(tmpbytes))
6667

67-
// Afero
68+
// osFS ==> BAD
6869
fmt.Println("Afero:")
6970
afs := &afero.Afero{Fs: osFS}
7071
fmt.Println(afs.ReadFile(filepath))
7172

72-
// IOFS ==> OK
73+
// BasePathFs ==> BAD
74+
fmt.Println("Afero:")
75+
basePathFs0 := &afero.Afero{Fs: afero.NewBasePathFs(osFS, "tmp")}
76+
fmt.Println(basePathFs0.ReadFile(filepath))
77+
78+
// IOFS ==> GOOD
7379
fmt.Println("IOFS:")
7480
ioFS := afero.NewIOFS(osFS)
7581
fmt.Println(ioFS.ReadFile(filepath))
7682
fmt.Println(ioFS.Open(filepath))
77-
}
7883

79-
func Beego() {
80-
beego.Router("/", &MainController{})
81-
beego.Run()
84+
// BasePathFs ==> GOOD
85+
fmt.Println("BasePathFs:")
86+
basePathFs := afero.NewBasePathFs(osFS, "tmp")
87+
fmt.Println(afero.ReadFile(basePathFs, filepath))
88+
afero.ReadFile(basePathFs, filepath)
8289
}
8390

8491
func Echo() {
@@ -104,41 +111,16 @@ func Fiber() {
104111
_ = app.Listen(":3000")
105112
}
106113

107-
func Fasthttp() {
108-
ln, _ := net.Listen("tcp4", "127.0.0.1:8080")
109-
requestHandler := func(ctx *fasthttp.RequestCtx) {
110-
filePath := ctx.QueryArgs().Peek("filePath")
111-
_ = ctx.Response.SendFile(string(filePath))
112-
ctx.SendFile(string(filePath))
113-
ctx.SendFileBytes(filePath)
114-
fileHeader, _ := ctx.FormFile("file")
115-
_ = fasthttp.SaveMultipartFile(fileHeader, string(filePath))
116-
fasthttp.ServeFile(ctx, string(filePath))
117-
fasthttp.ServeFileUncompressed(ctx, string(filePath))
118-
fasthttp.ServeFileBytes(ctx, filePath)
119-
fasthttp.ServeFileBytesUncompressed(ctx, filePath)
120-
}
121-
_ = fasthttp.Serve(ln, requestHandler)
122-
}
123-
func IrisTest() {
124-
app := iris.New()
125-
app.UseRouter(iris.Compression)
126-
app.Get("/", func(ctx iris.Context) {
127-
filepath := ctx.URLParam("filepath")
128-
_ = ctx.SendFile(filepath, "file")
129-
_ = ctx.SendFileWithRate(filepath, "file", 0, 0)
130-
_ = ctx.ServeFile(filepath)
131-
_ = ctx.ServeFileWithRate(filepath, 0, 0)
132-
_, _, _ = ctx.UploadFormFiles(filepath, beforeSave)
133-
_, fileHeader, _ := ctx.FormFile("file")
134-
_, _ = ctx.SaveFormFile(fileHeader, filepath)
114+
func IrisTest(ctx context.Context) {
115+
filepath := ctx.URLParam("filepath")
116+
_ = ctx.SendFile(filepath, "file")
117+
_ = ctx.SendFileWithRate(filepath, "file", 0, 0)
118+
_ = ctx.ServeFile(filepath)
119+
_ = ctx.ServeFileWithRate(filepath, 0, 0)
120+
_, _, _ = ctx.UploadFormFiles(filepath, nil)
121+
_, fileHeader, _ := ctx.FormFile("file")
122+
_, _ = ctx.SaveFormFile(fileHeader, filepath)
135123

136-
})
137-
app.Listen(":8080")
138-
139-
}
140-
func beforeSave(ctx iris.Context, file *multipart.FileHeader) bool {
141-
return true
142124
}
143125
func Gin() {
144126
router := gin.Default()

‎go/ql/test/library-tests/semmle/go/security/FileSystemAccess/vendor/github.com/beego/beego/v2/server/web/context/stub.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)