Skip to content

Commit 42980a1

Browse files
committed
Python: Model shelve.open
1 parent a81d359 commit 42980a1

File tree

2 files changed

+38
-2
lines changed

2 files changed

+38
-2
lines changed

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,42 @@ private module StdlibPrivate {
498498
override string getFormat() { result = "pickle" }
499499
}
500500

501+
// ---------------------------------------------------------------------------
502+
// shelve
503+
// ---------------------------------------------------------------------------
504+
/**
505+
* A call to `shelve.open`
506+
* See https://docs.python.org/3/library/shelve.html#shelve.open
507+
*
508+
* Claiming there is decoding of the input to `shelve.open` is a bit questionable, since
509+
* it's not the filename, but the contents of the file that is decoded.
510+
*
511+
* However, we definitely want to be able to alert if a user is able to control what
512+
* file is used, since that can lead to code execution (even if that file is free of
513+
* path injection).
514+
*
515+
* So right now the best way we have of modeling this seems to be to treat the filename
516+
* argument as being deserialized...
517+
*/
518+
private class ShelveOpenCall extends Decoding::Range, FileSystemAccess::Range,
519+
DataFlow::CallCfgNode {
520+
ShelveOpenCall() { this = API::moduleImport("shelve").getMember("open").getACall() }
521+
522+
override predicate mayExecuteInput() { any() }
523+
524+
override DataFlow::Node getAnInput() {
525+
result in [this.getArg(0), this.getArgByName("filename")]
526+
}
527+
528+
override DataFlow::Node getAPathArgument() {
529+
result in [this.getArg(0), this.getArgByName("filename")]
530+
}
531+
532+
override DataFlow::Node getOutput() { result = this }
533+
534+
override string getFormat() { result = "pickle" }
535+
}
536+
501537
// ---------------------------------------------------------------------------
502538
// popen2
503539
// ---------------------------------------------------------------------------

python/ql/test/library-tests/frameworks/stdlib/Decoding.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515

1616
# if the file opened has been controlled by an attacker, this can lead to code
1717
# execution. (underlying file format is pickle)
18-
shelve.open(filepath) # $ MISSING: decodeInput=filepath decodeOutput=shelve.open(..) decodeFormat=pickle decodeMayExecuteInput getAPathArgument=filepath
19-
shelve.open(filename=filepath) # $ MISSING: decodeInput=filepath decodeOutput=shelve.open(..) decodeFormat=pickle decodeMayExecuteInput getAPathArgument=filepath
18+
shelve.open(filepath) # $ decodeInput=filepath decodeOutput=shelve.open(..) decodeFormat=pickle decodeMayExecuteInput getAPathArgument=filepath
19+
shelve.open(filename=filepath) # $ decodeInput=filepath decodeOutput=shelve.open(..) decodeFormat=pickle decodeMayExecuteInput getAPathArgument=filepath
2020

2121
# TODO: These tests should be merged with python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py
2222
base64.b64decode(payload) # $ decodeInput=payload decodeOutput=base64.b64decode(..) decodeFormat=Base64

0 commit comments

Comments
 (0)