fix: improve dynamic member access fallback and dataflow continuity#113
fix: improve dynamic member access fallback and dataflow continuity#113Ris-1kd wants to merge 45 commits intoantgroup:mainfrom
Conversation
Ris-1kd
requested review from
AntJiuFo,
Arielwyy and
alipaydeshui
as code owners
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the Python analyzer's taint tracking and symbol resolution capabilities. Key changes include the addition of a new taint flow rule for SQL injection, improved support for list comprehensions through append semantics and small union iteration, and the implementation of fallback mechanisms for unresolved member calls and ORM-style object managers. The review feedback highlights opportunities to optimize receiver evaluation, improve the performance of global method searches, and increase the efficiency of list index calculations.
| return undefined | ||
| } | ||
|
|
||
| const receiver = this.processInstruction(scope, node.callee.object, state) |
There was a problem hiding this comment.
The receiver object node.callee.object has already been evaluated during the processing of node.callee (which calls processMemberAccess). Re-evaluating it here is redundant and could lead to incorrect analysis if the receiver expression has side effects (e.g., a function call). It is safer and more efficient to retrieve the already evaluated receiver from currentFclos.object if available.
| const receiver = this.processInstruction(scope, node.callee.object, state) | |
| const receiver = currentFclos?.object || this.processInstruction(scope, node.callee.object, state) |
| for (const modKey of Object.keys(modules)) { | ||
| const modScope = modules[modKey] | ||
| for (const key of Object.keys(modScope?.value || {})) { | ||
| const cls = modScope.value[key] | ||
| if (cls?.vtype !== 'class') continue | ||
| const method = cls?.value?.[prop.name] | ||
| if (method?.vtype === 'fclos') { | ||
| const methodCopy = _.clone(method) | ||
| methodCopy._this = receiver | ||
| methodCopy.parent = receiver | ||
| methodCopy.object = receiver | ||
| methodCopy._qid = `${receiver?.qid || receiver?.sid || key}.${prop.name}` | ||
| return methodCopy | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This global search for a method across all classes in all modules is computationally expensive. Since this fallback is triggered for unresolved member calls, it could significantly degrade performance in large projects. Consider implementing a global method index or caching the results of this search to improve efficiency.
| const numericIndices = Object.keys(container.value) | ||
| .filter((k) => /^\d+$/.test(k)) | ||
| .map((k) => Number(k)) | ||
| const nextIndex = numericIndices.length > 0 ? Math.max(...numericIndices) + 1 : 0 |
There was a problem hiding this comment.
Calculating the next numeric index by filtering all keys and finding the maximum value on every push operation results in Object.keys(container.value).length if the container is known to be a sequential list without holes.
| const numericIndices = Object.keys(container.value) | |
| .filter((k) => /^\d+$/.test(k)) | |
| .map((k) => Number(k)) | |
| const nextIndex = numericIndices.length > 0 ? Math.max(...numericIndices) + 1 : 0 | |
| const nextIndex = Object.keys(container.value).length |
1 participant
No description provided.