Skip to content

Conversation

@asgerf
Copy link
Contributor

@asgerf asgerf commented Mar 11, 2025

Attribute nodes were missing their enclosing callable, which means the shared data flow library was unable to track flow there. As far as I can tell this only affects Vue.

I'm honestly not sure why DataFlow::XmlAttributeNode exists since we don't seem to use it for anything 😕.

@github-actions github-actions bot added the JS label Mar 11, 2025
@asgerf asgerf force-pushed the js/vue-fix branch 2 times, most recently from b3be197 to 2c8da21 Compare March 11, 2025 11:49
@asgerf asgerf marked this pull request as ready for review March 11, 2025 15:55
Copilot AI review requested due to automatic review settings March 11, 2025 15:55
@asgerf asgerf requested a review from a team as a code owner March 11, 2025 15:55
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

A fix for Vue attribute nodes where attributes were missing an enclosing callable, ensuring that the shared data flow library now properly tracks the flow for the v-html attribute.

  • Updated change note file to document the bug fix.
  • Added inline test comments in the Vue test file to mark source and alert markers.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File Description
javascript/ql/src/change-notes/2025-03-11-vue-fix.md Added change notes for the bug fix
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/v-html.vue Updated test file with inline comments for source and alert markers

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

Copy link
Collaborator

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

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

Looks sensible, thanks for the clear explanation.

or
result = this.asLibraryCallable()
or
result = this.asFileCallable().toString()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I always worry about missing cases like this - this kind of pattern matching feels like a good candidate for a QL-for-QL query :)

@asgerf asgerf merged commit b4016c1 into github:main Mar 12, 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