Python: model os path file accesses #6741
Conversation
There was a problem hiding this comment.
There are a few more functions that could reveal information about the file system, by telling the last modification time of a file, e.g.
getmtime. This will raise an error if the file does not exist, so could also reveal the existence of files depending on the applications error handling. These are currently not included.
Can we please model these as well? they fit the FileSystemAccess concept as far as I can tell.
on non-existing files.
Co-authored-by: Rasmus Wriedt Larsen <rasmuswriedtlarsen@gmail.com>
|
There was already a taint-step for |
| private string pathComputation() { | ||
| result in [ | ||
| "abspath", "basename", "commonpath", "dirname", "expanduser", "expandvars", "join", | ||
| "normcase", "normpath", "realpath", "relpath", "split" | ||
| ] |
There was a problem hiding this comment.
| private string pathComputation() { | |
| result in [ | |
| "abspath", "basename", "commonpath", "dirname", "expanduser", "expandvars", "join", | |
| "normcase", "normpath", "realpath", "relpath", "split" | |
| ] | |
| private string pathComputation() { | |
| result in [ | |
| "abspath", "basename", "commonpath", "commonprefix", "dirname", "expanduser", "expandvars", "join", | |
| "normcase", "normpath", "realpath", "relpath", "split", "splitdrive", "splitext" | |
| ] |
or was there a reason not to add commonprefix? I'm wondering if you did not include it since it might be used as a form of sanitizer? -- I don't feel doubt about splitdrive and splitext should have taint-steps
NIT: Since this predicate is only used once (in the char-pred of OsPathComputation), could we please inline it there?
There was a problem hiding this comment.
The reasons for not including these are given in the list "Functions that need summaries". These functions all do not map tainted input to tainted output but rather map some content to some content.
| // Functions that need summaries: | ||
| // - os.path.commonpath(paths): takes a sequence | ||
| // - os.path.commonprefix(list): takes a list argument | ||
| // - os.path.splitdrive: retunrs a tuple | ||
| // - os.path.splittext: returns a tuple |
There was a problem hiding this comment.
This list is incomplete (for example, missing split). Since all our current taint-modeling would need proper flow-summaries in the future, I would suggest we remove this, and deal with it once we remodel things. it just seems a bit too brittle to my liking 😐
| // Functions that need summaries: | |
| // - os.path.commonpath(paths): takes a sequence | |
| // - os.path.commonprefix(list): takes a list argument | |
| // - os.path.splitdrive: retunrs a tuple | |
| // - os.path.splittext: returns a tuple |
This PR includes a number of functions in
os.pathinto the conceptFileSystemAccess. These all test for the existence of files in some way.There are a few more functions that could reveal information about the file system, by telling the last modification time of a file, e.g.
getmtime. This will raise an error if the file does not exist, so could also reveal the existence of files depending on the applications error handling. These are currently not included.