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

C/C++: Paths reported in sarif results contain extra back slashes in latest version of CodeQL (2.19.2) #17972

Open
jacob-ronstadt opened this issue Nov 12, 2024 · 2 comments
Assignees
Labels
bug Something isn't working question Further information is requested

Comments

@jacob-ronstadt
Copy link

Description of the issue

Expr used as source in data flow. Source is a string that doesn't match a given pattern:
e.getValue().toString().toLowerCase().matches(pattern)

Source is used in the output message:
select f, message, source, source.asIndirectExpr().toString()

In the source code the string is a function argument hard-coded such as: L"\\some\\bad\\path\\test\\test.txt"

In the sarif file results from running codeql database analyze with --format=sarif-latest, the same source code, and using the same commands for building and analyzing the database, previous versions of CodeQL CLI (2.17.6 tested) show this as:
"message":{"text":"\\some\\bad\\path\\test\\test.txt"}}]}],
while CodeQL CLI 2.19.2 show:
"message":{"text":"\\\\some\\\\bad\\\\path\\\\test\\\\test.txt"}}]}],

@jacob-ronstadt jacob-ronstadt added the question Further information is requested label Nov 12, 2024
@smowton smowton changed the title Paths reported in sarif results contain extra back slashes in latest version of CodeQL (2.19.2) C/C++: Paths reported in sarif results contain extra back slashes in latest version of CodeQL (2.19.2) Nov 12, 2024
@jketema
Copy link
Contributor

jketema commented Nov 13, 2024

Hi @jacob-ronstadt,

Thanks for the report. We're currently discussing internally whether this change is correct or not.

@aeisenberg aeisenberg added the bug Something isn't working label Nov 22, 2024
@aeisenberg
Copy link
Contributor

For reference, it looks like when we fixed #15245, we inadvertently introduced this issue, related to this part of the SARIF spec: https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790717. If I'm understanding correctly, only text inside of links should be double escaped. All other places should be escaped once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants