Python: Fix iterable-unpacking taint CP

[RasmusWL]

When running ql/python/ql/src/Security/CWE-079/ReflectedXss.ql against the database for flask.

Iitially there were 10 million result-tuples for iterable_unpacking_descent.

With this change, we're down to roughly 2100.

Fixes the problem introduced in #2700, and disabled in #2973

[RasmusWL]

Started https://jenkins.internal.semmle.com/job/Changes/job/Python-Differences/33/

[tausbn]

One minor comment/question, otherwise I think this is okay. I stared at it for a while, and I couldn't see any obvious problems (but I'm not sure I trust that there are none based on this! 😬).
I'm curious to see how the performance tests pan out.
I would also like to see the RA generated/executed on a large snapshot for the predicates involved (i.e. the main predicate and the two helper predicates).

[tausbn]

Should this have a depth > 1 constraint (so that the three disjuncts are indeed disjoint)?

[RasmusWL]

Fixed! This won't change the results, just the executions that's that we have to evaluate 👍

[RasmusWL]

I'm curious to see how the performance tests pan out.

https://jenkins.internal.semmle.com/job/Changes/job/Python-Differences/33/ looks fine. Are you asking for a dist-compare besides this?

I would also like to see the RA generated/executed on a large snapshot for the predicates involved (i.e. the main predicate and the two helper predicates).

Alright. Is saltstack/salt large enough for you with ~600k lines of python code? and I think I need some guidance on the RA generated/executed since I wouldn't know exactly which parts to pick from the query logs.

[BekaValentine]

This looks good. I especially like the long comment with an explanatory example, it helps put the implementation into a context of intention.

[RasmusWL]

I ran ReflectedXss.ql on the current snapshot for saltstack/salt, and I didn't see any problems. (this I the same query I used doing my other tests)

I created a gist with the query logs, but that only shows the first 8 thousand lines, so you need to look at the raw results to see all 56k lines 😉

Looking at all the places it says iterable_unpacking_descent I didn't see anything of concern.

Is this sort of what you were asking for @tausbn? -- when you said:

I would also like to see the RA generated/executed on a large snapshot for the predicates involved (i.e. the main predicate and the two helper predicates).

[tausbn]

Alright. Is saltstack/salt large enough for you with ~600k lines of python code? and I think I need some guidance on the RA generated/executed since I wouldn't know exactly which parts to pick from the query logs.

Sure, salt should be plenty big. What I meant by the RA would amount to:

1. Running an appropriate query, generating a log with tuple counts (since this has to do with taint tracking, I imagine most taint tracking queries would do).
2. Looking for the place in the log where the iterable_unpacking_descent and taint_at_depth predicates are evaluated, to see whether they result in a "reasonable" amount of tuples, whether there are any bad join orders or cartesian products, etc. Basically this amounts to searching for those names until you find "tuple counts for " followed by the name of the predicate.

For 1. you'll most likely want to use a recent version of VSCode, with the most recent improvements to log handling. (Or you could also just run it manually.)

[tausbn]

Oh, hah. I was in the process of writing my comment when you added yours. I'll take a quick look at the results.

[tausbn]

RA and tuple counts look fine to me. Merging.