Skip to content

add support for ESM; add check for newExpr; complement taint matching#121

Open
CyanM0un wants to merge 1 commit intoantgroup:mainfrom
CyanM0un:main
Open

add support for ESM; add check for newExpr; complement taint matching#121
CyanM0un wants to merge 1 commit intoantgroup:mainfrom
CyanM0un:main

Conversation

@CyanM0un
Copy link
Copy Markdown
Collaborator

  • add support for ESM: check if corresponding .ts file exists
  • add check for newExpr: consider this as sink
  • complement taint matching: directly compare fsig

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the taint analysis engine by adding signature-based source marking and sink checking for NewExpression nodes. It also improves file path resolution in the JavaScript analyzer to better handle TypeScript files and index lookups. Feedback focuses on ensuring cross-platform compatibility for path operations and verifying the availability of the call.name property on AST nodes.

}
break
}
} else if (call.name === tspec.fsig) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The property call.name is likely undefined for standard CallExpression nodes in the UAST. Typically, the function name is accessed via call.callee.name (for Identifiers) or call.callee.property.name (for MemberAccess), both of which are already handled in the preceding blocks. If the intention is to match a resolved full signature (as suggested by the PR title), call.name will not work unless the analyzer explicitly populates this property on the AST node. Consider extracting the resolved signature from the closure or using a unified naming utility.

Comment on lines +851 to +852
cwd = path.join(pathname, '../')
filename = pathname.split('/').pop()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using pathname.split('/').pop() is not cross-platform safe and may fail on systems using different path separators (e.g., Windows). It is recommended to use path.basename(pathname) and path.dirname(pathname) for robust path manipulation. Additionally, the logic for globbing and path resolution is duplicated in the else if block (lines 861-863), which could be refactored for better maintainability.

Suggested change
cwd = path.join(pathname, '../')
filename = pathname.split('/').pop()
cwd = path.dirname(pathname)
filename = path.basename(pathname)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant