CodeQL Not catching known CWE's, not failing build for caught CWE

[myoung34]

Got them to run as expected per PR #104

However this pr should definitely cause a failure for:

• CWE-377 py/insecure-temporary-file - via "/opt/hostedtoolcache/CodeQL/0.0.0-20200630/x64/ql/python/ql/src/Security/CWE-377/InsecureTemporaryFile.ql" : { },
• CWE-78 py/command-line-injection - via "/opt/hostedtoolcache/CodeQL/0.0.0-20200630/x64/ql/python/ql/src/Security/CWE-078/CommandInjection.ql" : { },

It did catch CWE-377, but it did not fail the build:

It also didnt show much info. I did not realize it caught this until I accidentally clicked on the changes:

It does show as a check, but only if you happen to expand it.
The default view:

If I expand it:

This is very very easy to miss and not at all a good UX. I would think that the action itself would fail the build, or that the check would be easier to spot a failure. At a glance the PR looks mergeable.

That said, it still never caught CWE-78

[myoung34]

Update: Much later (no changes on my side) it now shows a failed check to the PR: which is very odd since it wasnt like this for a few hours after i submitted this.

That aside, I'm still a bit curious why CWE-78 isnt triggered

[robertbrignull]

@tausbn, do you know why myoung34/tilty#14 didn't trigger https://github.com/github/codeql/blob/master/python/ql/src/Security/CWE-078/CommandInjection.ql?

My guess is that it isn't detecting the source. My what I can tell that query is explicitly looking for HttpRequestTaintSource and this doesn't include the click library which is for command lines.

[tausbn]

Yeah, I think you've got it exactly right. We don't currently model click (or any other command line libraries, to my knowledge), so we don't have the necessary sources for the taint tracking to work here.

[RasmusWL]

The problem highlighted could only ever trigger https://github.com/github/codeql/blob/df3eb9f9c54335b7d0991ff05451692593982442/python/ql/src/Security/CWE-094/CodeInjection.ql, since exec is used (which executes python code, not external programs)

I'm not convinced it's a good example though. The "evil" user that can trigger the exec clearly has access to invoke the program from the command line, so from my understanding the "evil" user could just execute the code with python -c "<command>". (I might be mistaking though)

[myoung34]

I'm not convinced it's a good example though. The "evil" user that can trigger the exec clearly has access to invoke the program from the command line, so from my understanding the "evil" user could just execute the code with python -c "". (I might be mistaking though)

The example in question is purely to beta test codeql catches.
I'm investigating using this at our company, so I was hoping to see how the workflows worked in terms of UI/UX and noticed that CWE-78 didn't trigger (after my original issues with CWE-377 started working at some point)

Likely i wrote my example too fast and used exec() vs what i really meant: subprocess.call(executable)

[myoung34]

I'll reopen if this pops back up but it seems to be working as expected now