Python: Pycurl SSL Disabled #16812
Python: Pycurl SSL Disabled #16812
Conversation
|
Hello porcupineyhairs 👋 In the meantime, feel free to make changes to the pull request. If you'd like to maximize payout for your this and future submissions, here are a few general guidelines, that we might take into consideration when reviewing a submission.
Please note that these are guidelines, not rules. Since we have a lot of different types of submissions, the guidelines might vary for each submission. Happy hacking! |
| exists(IntegerLiteral i | i.getN() = "0" and c.getArg(1).asExpr() = i) | ||
| | | ||
| disablingNode = c and argumentOrigin = c.getArg(1) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
This class is representing calls to setopt where the first argument is specifically "PyCurl.URL". For these calls, the current implementation of disablesCertificateValidation as none() is correct. What you have done is expressing that this call disables certificate validation if there is another call with arguments (PyCurl.SSL_VERIFYPEER, 0).
What you would need to do is likely to create a new class extending Http::Client::Request::Range representing calls to setopt where the first argument is specifically "PyCurl.SSL_VERIFYPEER" and then you can implement disablesCertificateValidation by comparing the second argument with 0 (and also with False, please).
There was a problem hiding this comment.
Closer, but you are still looking at another call (c = setopt().getACall()). You have to look at the argument for the call represented by the class; something like c = this.getACall(). Also, the boolean has to come from the call; you are now just checking if they wrote false anywhere in the code.
There was a problem hiding this comment.
@yoff Changes done! PTAL.
There was a problem hiding this comment.
Yes, much better :-) I think the boolean would also have to go into arg(1)?
| /** Gets a reference to an instance of `pycurl.Curl.SSL_VERIFYPEER`. */ | ||
| private API::Node sslverifypeer() { | ||
| result = API::moduleImport("pycurl").getMember("SSL_VERIFYPEER") | ||
| } |
There was a problem hiding this comment.
This can also come from an instance, see http://pycurl.io/docs/latest/curlobject.html#pycurl.Curl.setopt.
| exists(API::CallNode c | | ||
| c = setopt().getACall() and | ||
| sslverifypeer().getAValueReachableFromSource() = c.getArg(0) and | ||
| exists(IntegerLiteral i | i.getN() = "0" and c.getArg(1).asExpr() = i) |
There was a problem hiding this comment.
It is possible to just compare the integer values
| exists(IntegerLiteral i | i.getN() = "0" and c.getArg(1).asExpr() = i) | |
| exists(IntegerLiteral i | i.getValue() = 0 and c.getArg(1).asExpr() = i) |
|
Hi @porcupineyhairs - we would like to close the last remaining bug bounty PRs soon. Do you think you will keep on working on this PR? |
porcupineyhairs
force-pushed
the
pyloadSsl
branch
from
01c58d2 to
e2dd126
Compare
|
@sylwia-budzynska Sorry for the delay. Last couple of weeks have been busy. I will get this cleared soon. @yoff I have made the changes now. PTAL. However, for some reason, Codeql is giving me duplicate results when I run the original query. |
| exists(IntegerLiteral i | i.getN() = "0" and c.getArg(1).asExpr() = i) | ||
| | | ||
| disablingNode = c and argumentOrigin = c.getArg(1) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Yes, much better :-) I think the boolean would also have to go into arg(1)?
| exists(IntegerLiteral i | i.getValue() = 0 and this.getArg(1).asExpr() = i) | ||
| or | ||
| exists(BooleanLiteral b | b.booleanValue() = false and this.getArg(_).asExpr() = b) |
There was a problem hiding this comment.
| exists(IntegerLiteral i | i.getValue() = 0 and this.getArg(1).asExpr() = i) | |
| or | |
| exists(BooleanLiteral b | b.booleanValue() = false and this.getArg(_).asExpr() = b) | |
| this.getArg(1).asExpr().(IntegerLiteral).getValue() = 0 | |
| or | |
| this.getArg(1).asExpr().(BooleanLiteral).booleanValue() = false |
I think it is easier to read without the exists, but the important change here is that for the boolean value, we also only look at arg(1).
Pycurl is a library which provides curl binding in python. The original library is partially modelled in codeql. This PR adds support to test for SSL certificate validation when using pycurl.