sarif-rs icon indicating copy to clipboard operation
sarif-rs copied to clipboard

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

Open igrr opened this issue 9 months ago • 0 comments

This is probably an issue I introduced in https://github.com/psastras/sarif-rs/pull/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.

igrr avatar May 21 '24 15:05 igrr