Skip to content

Python: Add modeling of simplejson/ujson/idna #5864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 19, 2021

Conversation

RasmusWL
Copy link
Member

These could all have gotten their own PR, but I thought they were similar enough to just keep it as one PR 🤷

RasmusWL added 7 commits May 10, 2021 10:55
I noticed that we don't handle PostUpdateNote very well in the concept tests,
for exmaple for `json.dump(...)` there _should_ have been an `encodeOutput` as
part of the inline expectations.

I'll work on fixing that up in a separate PR, to keep things clean.
Inspired by the work on previous commit
The problem with `tainted_filelike` not having taint, is that in the call

`ujson.dump(tainted_obj, tainted_filelike)`

there is no PostUpdateNote for `tainted_filelike` :( The reason is that
points-to is not able to resolve the call, so none of the clauses in
`argumentPreUpdateNode` matches

See https://github.com/github/codeql/blob/08731fc6cf4ba6951cd4e8f239eac1f3388d3957/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll#L101-L111

Let's deal with that issue in an other PR though
@RasmusWL RasmusWL requested a review from a team as a code owner May 10, 2021 13:12
Which I had done locally. Problem is the same about not having PostUpdateNode
when points-to is not able to resolve the call, so I'm happy to just make CI
happy right now, and hopefully we'll get a fix to the underlying problem soon 😊
Copy link
Contributor

@tausbn tausbn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I have a single comment but it's somewhat outside the scope of this PR, and probably best left for a different PR. Accordingly, I have marked this PR as approved. 👍

@RasmusWL
Copy link
Member Author

:shipit:

@codeql-ci codeql-ci merged commit 23e8092 into github:main May 19, 2021
@RasmusWL RasmusWL deleted the some-framework-modeling branch May 19, 2021 09:31
RasmusWL added a commit to RasmusWL/codeql that referenced this pull request May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants