Golang : Add query to detect JWT signing vulnerabilities

[porcupineyhairs]

Using a hard-coded secret for signing JWT tokens in an open source project can cause security vulnerabilities. This PR includes a query to find instances where a hard-coded string is used to sign a token.

[owen-mc]

Thanks for this contribution. I haven't looked at it closely yet, but my first comment is that it would benefit from some tests and a qhelp file.

[porcupineyhairs]

@owen-mc added QHelp along with an example. PTAL

[porcupineyhairs]

@pwntester I thought about handling the FP in the first case. Having a empty string "" as a secret is a valid detection as it would the JWT is not signed at all. Hence, excluding all empty string from taint flow would lead to a lot of false negatives. However, I think considering any kind of comparison operation as a sanitizer would work here. So I have added a new sanitizer to detect any kind of binary op such as == or !=. I don't go about reasoning the correctness of the operation here.

In the second case, the project has a default insecure hard-coded key which it uses in the absence of a user supplied key. The alert is for the default key flow. Since, the default configuration is insecure, shouldn't we be flagging it? Ideally, the project should generate and use a random key if a user does not supply a custom key. Hence, I have kept this as is.

As for the clause you mentioned earlier, I think it a QL bug. See my other code comment for details.

[porcupineyhairs]

@pwntester RE : https://github.com/github/codeql-go/pull/705#discussion_r837331953

In the firstcase, I was talking about the general case where there is no key used to sign a JWT. Since there is no secret, there is no encryption here.

As for the second case, can't you find kiali as the default string, QL gives me the flow directly on running no changes required.

[porcupineyhairs]

Hey, Please don't run this on LGTM until this issue is resolved.

[owen-mc]

Test failure:

FAILED: /home/runner/work/codeql-go/codeql-go/ql/test/experimental/CWE-321/CONSISTENCY/UnexpectedFrontendErrors.ql

which means that it thinks there's something wrong with the go code in ql/test/experimental/CWE-321

[porcupineyhairs]

@owen-mc I don't understand why the tests are failing. The build successfully completes on my PC.

[porcupineyhairs]

I think I was missing a main function. The tests are now cleared.

@owen-mc I think this is now ready for a review. PTAL.

[porcupineyhairs]

@owen-mc Any updates here?

[pwntester]

hi @porcupineyhairs We are waiting for the CodeQL fixes to be pushed to LGTM to run the query again. In the meantime, I run the query on a smaller set of projects and found a FP worth addressing

[porcupineyhairs]

@owen-mc A quick update: I have seen your comments and plan on applying them. I have found a few false positives which the query as of now detects. I plan on adding code to reduce them soon. I will push the changes once I am done. Thank you for waiting.

[owen-mc]

@smowton will take over reviewing this PR.

[porcupineyhairs]

I am closing this PR and opening a new one github/codeql#9378