Skip to content

Commit 7483075

Browse files
committed
Python: Fixup modeling of os.open
1 parent d245db5 commit 7483075

File tree

3 files changed

+11
-6
lines changed

3 files changed

+11
-6
lines changed

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ module StdlibPrivate {
338338
* Modeling of path related functions in the `os` module.
339339
* Wrapped in QL module to make it easy to fold/unfold.
340340
*/
341-
private module OsFileSystemAccessModeling {
341+
module OsFileSystemAccessModeling {
342342
/**
343343
* A call to the `os.fsencode` function.
344344
*
@@ -395,7 +395,7 @@ module StdlibPrivate {
395395
*
396396
* See https://docs.python.org/3/library/os.html#os.open
397397
*/
398-
private class OsOpenCall extends FileSystemAccess::Range, DataFlow::CallCfgNode {
398+
class OsOpenCall extends FileSystemAccess::Range, DataFlow::CallCfgNode {
399399
OsOpenCall() { this = os().getMember("open").getACall() }
400400

401401
override DataFlow::Node getAPathArgument() {
@@ -1501,7 +1501,12 @@ module StdlibPrivate {
15011501
private class OpenCall extends FileSystemAccess::Range, Stdlib::FileLikeObject::InstanceSource,
15021502
ThreatModelSource::Range, DataFlow::CallCfgNode
15031503
{
1504-
OpenCall() { this = getOpenFunctionRef().getACall() }
1504+
OpenCall() {
1505+
this = getOpenFunctionRef().getACall() and
1506+
// when analyzing stdlib code for os.py we wrongly assume that `os.open` is an
1507+
// alias of the builtins `open` function
1508+
not this instanceof OsFileSystemAccessModeling::OsOpenCall
1509+
}
15051510

15061511
override DataFlow::Node getAPathArgument() {
15071512
result in [this.getArg(0), this.getArgByName("file")]

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ def test_fspath():
8787
os.fspath(path=TAINTED_STRING), # $ tainted
8888
)
8989

90-
os.open("path", os.O_RDONLY) # $ getAPathArgument="path" SPURIOUS: threatModelSource[file]=os.open(..)
91-
os.open(path="path", flags=os.O_RDONLY) # $ getAPathArgument="path" SPURIOUS: threatModelSource[file]=os.open(..)
90+
os.open("path", os.O_RDONLY) # $ getAPathArgument="path"
91+
os.open(path="path", flags=os.O_RDONLY) # $ getAPathArgument="path"
9292

9393
os.access("path", os.R_OK) # $ getAPathArgument="path"
9494
os.access(path="path", mode=os.R_OK) # $ getAPathArgument="path"

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
open("foo").readline(), # $ tainted threatModelSource[file]=open(..) getAPathArgument="foo"
5959
open("foo").readlines(), # $ tainted threatModelSource[file]=open(..) getAPathArgument="foo"
6060

61-
os.read(os.open("foo"), 1024), # $ tainted threatModelSource[file]=os.read(..) SPURIOUS: threatModelSource[file]=os.open(..) getAPathArgument="foo"
61+
os.read(os.open("foo"), 1024), # $ tainted threatModelSource[file]=os.read(..) getAPathArgument="foo"
6262
)
6363

6464
########################################

0 commit comments

Comments
 (0)