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 generated by clang-tidy-sarif contains duplicate note items #638

Open
igrr opened this issue May 21, 2024 · 0 comments
Open

SARIF generated by clang-tidy-sarif contains duplicate note items #638

igrr opened this issue May 21, 2024 · 0 comments

Comments

@igrr
Copy link
Contributor

igrr commented May 21, 2024

This is probably an issue I introduced in #406.

I ran into a case where clang-tidy produces the following warnings:

/__w/idf-extra-components/idf-extra-components/freetype/freetype/src/base/ftbitmap.c:123:9: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
  123 |         FT_MEM_COPY( target->buffer, source->buffer,
      |         ^
/__w/idf-extra-components/idf-extra-components/freetype/freetype/include/freetype/internal/ftmemory.h:238:11: note: expanded from macro 'FT_MEM_COPY'
  238 |           ft_memcpy( dest, source, (FT_Offset)(count) )
      |           ^~~~~~~~~
/__w/idf-extra-components/idf-extra-components/freetype/freetype/include/freetype/config/ftstdlib.h:92:21: note: expanded from macro 'ft_memcpy'
   92 | #define ft_memcpy   memcpy
      |                     ^~~~~~
/__w/idf-extra-components/idf-extra-components/freetype/freetype/src/base/ftbitmap.c:123:9: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
  123 |         FT_MEM_COPY( target->buffer, source->buffer,
      |         ^
/__w/idf-extra-components/idf-extra-components/freetype/freetype/include/freetype/internal/ftmemory.h:238:11: note: expanded from macro 'FT_MEM_COPY'
  238 |           ft_memcpy( dest, source, (FT_Offset)(count) )
      |           ^~~~~~~~~
/__w/idf-extra-components/idf-extra-components/freetype/freetype/include/freetype/config/ftstdlib.h:92:21: note: expanded from macro 'ft_memcpy'
   92 | #define ft_memcpy   memcpy
      |                     ^~~~~~

When we convert this clang-tidy output into SARIF using clang-tidy-sarif and try to upload the resulting SARIF file to Github CodeQL, Github SARIF linter complains that:

Unable to upload "results.sarif" as it is not valid SARIF:
- instance.runs[0].results[7].relatedLocations contains duplicate item
...

which makes sense, since there are two notes for ftmemory.h:238:11 location.

Clang-tidy output looks quite odd here, I don't understand why it repeats the original message (Call to function 'memcpy' is insecure) as a note. However, there might be more cases where there could be more than one note pointing to the same line.

I guess we need to combine or deduplicate note messages pointing to the same location.

I'll try to find time to submit a PR, but for now I am just opening the issue to not forget about this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant