feat: Codex vuln detector - devtooligan updates #1499
Conversation
|
Additional pr based on this branch submitted. #1498 |
| prompt = "Is there a vulnerability in this solidity contracts?\n" | ||
| if self.slither.codex_contracts != "all" and contract.name not in self.slither.codex_contracts.split(","): | ||
| continue | ||
| prompt = "Analyze this Solidity contract and find the vulnerabilities. If you find any vulnerabilities, begin the response with {}".format(VULN_FOUND) |
There was a problem hiding this comment.
We could probably parameterize the prompt.
Also, maybe we could just always display the Codex response, even if it doesn't find anything. This would simplify it so we don't need adjust the prompt or look for a special word in the response that indicates something was found.
| res = self.generate_result(info) | ||
| results.append(res) | ||
|
|
||
| logging.info("Querying OpenAI") |
There was a problem hiding this comment.
I wasn't 100% sure what to do with logging/printing . I noticed in this file we used the logger but it didn't display anything when Slither was run. And I noticed in other parts of the tool we use print.
| print("Querying OpenAI") | ||
| answer = "" | ||
| res = {} | ||
| try: |
There was a problem hiding this comment.
I added this try/catch because I was hitting the max token limit with a large contract in the prompt. Another reason to consider running this per-function.
| @@ -52,29 +53,63 @@ def _detect(self) -> List[Output]: | |||
| openai.api_key = api_key | |||
|
|
|||
| for contract in self.compilation_unit.contracts: | |||
| prompt = "Is there a vulnerability in this solidity contracts?\n" | |||
There was a problem hiding this comment.
Above in line 40, the import openai inside of this function was problematic when I tried to extract the OpenAI query logic into a separate function. I think it scopes the import to the function so I wasn't able to access it.
Maybe we can move the import (wrapped in a try/except) to the top level? We could still do something here that checks whether it's been installed.
devtooligan
changed the title
devtooligan
changed the title
|
This is awesome, thanks @devtooligan |
|
I merged the improvements in #1498, some notes:
|
Highlights of this pr:
model,temperature, andmax_tokens--codex-contractsargument which takes a comma-delimited list of contracts and, if specified, it will limit the Codex detector to only those contracts. I found this a great way to save money when working with a large contract with a deeply nested inheritance tree. Someday, may want to expand this feature to all detectors.I did spend some time implementing a "retries" system. But for some reason, the retried "queries" were all identical. I guess I was hoping it would exhibit similar behavior to the chatbot when "retry" is pressed, which results in a different response. So I've removed that feature but will continue to look into it.