Skip to content

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 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 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