Skip to content

Commit d493cfd

Browse files
committed
Python: Model FastAPI FileResponse as FileSystemAccess
This was an oversight from our initial FastAPI modeling work.
1 parent 8c9e817 commit d493cfd

File tree

3 files changed

+19
-3
lines changed

3 files changed

+19
-3
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* Extended the modeling of FastAPI such that `fastapi.responses.FileResponse` are considered `FileSystemAccess`, making them sinks for the _Uncontrolled data used in path expression_ (`py/path-injection`) query.

python/ql/lib/semmle/python/frameworks/FastApi.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,17 @@ private module FastApi {
226226
}
227227
}
228228

229+
/**
230+
* A direct instantiation of a FileResponse.
231+
*/
232+
private class FileResponseInstantiation extends ResponseInstantiation, FileSystemAccess::Range {
233+
FileResponseInstantiation() { baseApiNode = getModeledResponseClass("FileResponse") }
234+
235+
override DataFlow::Node getAPathArgument() {
236+
result in [this.getArg(0), this.getArgByName("path")]
237+
}
238+
}
239+
229240
/**
230241
* An implicit response from a return of FastAPI request handler.
231242
*/
@@ -256,7 +267,8 @@ private module FastApi {
256267
* An implicit response from a return of FastAPI request handler, that has
257268
* `response_class` set to a `FileResponse`.
258269
*/
259-
private class FastApiRequestHandlerFileResponseReturn extends FastApiRequestHandlerReturn {
270+
private class FastApiRequestHandlerFileResponseReturn extends FastApiRequestHandlerReturn,
271+
FileSystemAccess::Range {
260272
FastApiRequestHandlerFileResponseReturn() {
261273
exists(API::Node responseClass |
262274
responseClass.getAUse() = routeSetup.getResponseClassArg() and
@@ -265,6 +277,8 @@ private module FastApi {
265277
}
266278

267279
override DataFlow::Node getBody() { none() }
280+
281+
override DataFlow::Node getAPathArgument() { result = this }
268282
}
269283

270284
/**

python/ql/test/library-tests/frameworks/fastapi/response_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,10 @@ async def file_response(): # $ requestHandler
136136

137137
# We don't really have any good QL modeling of passing a file-path, whose content
138138
# will be returned as part of the response... so will leave this as a TODO for now.
139-
resp = fastapi.responses.FileResponse(__file__) # $ HttpResponse
139+
resp = fastapi.responses.FileResponse(__file__) # $ HttpResponse getAPathArgument=__file__
140140
return resp # $ SPURIOUS: HttpResponse mimetype=application/json responseBody=resp
141141

142142

143143
@app.get("/file_response2", response_class=fastapi.responses.FileResponse) # $ routeSetup="/file_response2"
144144
async def file_response2(): # $ requestHandler
145-
return __file__ # $ HttpResponse
145+
return __file__ # $ HttpResponse getAPathArgument=__file__

0 commit comments

Comments
 (0)