Skip to content

JS: Added support for fastify.addHook #19300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 29, 2025
Merged

JS: Added support for fastify.addHook #19300

merged 8 commits into from
Apr 29, 2025

Conversation

Napalys
Copy link
Contributor

@Napalys Napalys commented Apr 14, 2025

The following pull-request adds support for fastify's addHook method.

@Napalys Napalys marked this pull request as ready for review April 14, 2025 12:10
@Copilot Copilot AI review requested due to automatic review settings April 14, 2025 12:10
@Napalys Napalys requested a review from a team as a code owner April 14, 2025 12:10
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for Fastify's addHook method to expand CodeQL's query tests for code injection vulnerabilities. Key changes include:

  • Introducing test cases for various Fastify hook events using eval on request query parameters.
  • Adding a corresponding model extension in fastify.model.yml.
  • Updating change notes to document added support for addHook.

Reviewed Changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.

File Description
javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js Adds multiple hook examples that demonstrate code injection scenarios via eval.
javascript/ql/lib/ext/fastify.model.yml Introduces a model extension mapping for the fastify addHook method.
javascript/ql/lib/change-notes/2025-04-14-fastify-addhook.md Documents the minor analysis change for fastify addHook support.
Files not reviewed (2)
  • javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected: Language not supported
  • javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected: Language not supported

@Napalys Napalys marked this pull request as draft April 14, 2025 12:20
@github github deleted a comment from Copilot AI Apr 14, 2025
@Napalys Napalys marked this pull request as ready for review April 23, 2025 14:26
@asgerf
Copy link
Contributor

asgerf commented Apr 28, 2025

Looking at the latest DCA run, it looks like we spuriously mark the return value of addHook as a reflected XSS sink. Could you update ResponseSendArgument in Fastify.qll to exclude addHook?

@Napalys Napalys merged commit 6de38b1 into github:main Apr 29, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants