Skip to content
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

SARIF: Backslash in query message is not escaped #15245

Closed
Marcono1234 opened this issue Jan 6, 2024 · 4 comments
Closed

SARIF: Backslash in query message is not escaped #15245

Marcono1234 opened this issue Jan 6, 2024 · 4 comments
Assignees
Labels
acknowledged GitHub staff acknowledges this issue bug Something isn't working

Comments

@Marcono1234
Copy link
Contributor

Marcono1234 commented Jan 6, 2024

Version

CodeQL extension version: 1.11.0 
CodeQL CLI version: 2.15.5 
Platform: win32 x64

(I tested this with the VS Code extension, but I assume the underlying issue is in the CodeQL CLI)

Description

Backslashes (\) in the query message are apparently not properly escaped in SARIF. This causes for example issues when viewing the alerts in the VS Code extension.

This seems to affect

  • the main message, e.g.:
    select t, "message: \\$@", t, "link"
    becomes "text" : "message: \\[link](1)" (should be \\\\[ in JSON)
  • placeholder messages, e.g.:
    select t, "message: $@", t, "link\\"
    becomes "text" : "message: [link\\](1)" (should be \\\\] in JSON)

See also SARIF spec 2.1.0 §3.11.6 Messages with embedded links.

How to reproduce

  1. Run the following Java query in VS Code (either on a local database or with variant analysis)
    /**
     * @kind problem
     */
    
    import java
    
    from TopLevelClass t
    where t.fromSource()
    select t, "message: $@", t, ["1", "\\2", "3\\", "4\\\\", "5[", "6]", "7\\[", "8\\]"]
  2. Have a look at the rendered results
    ❌ Bug: It contains multiple malformed results
    VS Code alerts view
  3. In the "Query History", right click the query execution and select "View Alerts (SARIF)"
    ❌ Bug: You will see in the SARIF data that the \ has not been properly escaped in the embedded links in "text"
@Marcono1234 Marcono1234 added the question Further information is requested label Jan 6, 2024
@MathiasVP
Copy link
Contributor

Hi @Marcono1234,

Thanks for raising this issue! We'll take a close look at this soon 🙂

@MathiasVP MathiasVP added the acknowledged GitHub staff acknowledges this issue label Jan 9, 2024
@aeisenberg
Copy link
Contributor

Assigning this to @dbartol who is looking at this area anyway and may be able to pick this up if it fits in with other work.

@aeisenberg aeisenberg added bug Something isn't working and removed question Further information is requested labels Apr 1, 2024
@dbartol dbartol closed this as completed Jul 11, 2024
@dbartol
Copy link
Contributor

dbartol commented Jul 11, 2024

The SARIF output from the CLI will be fixed in the 2.18.1 release.

@Marcono1234
Copy link
Contributor Author

Thanks! Is it intentional that changes such as this one (or #16880) are not mentioned in https://github.com/github/codeql-cli-binaries/blob/main/CHANGELOG.md?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged GitHub staff acknowledges this issue bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants