SARIF output from Clang Static Analyzer

[aido]

Hi,

It would be nice if the Clang Static Analyzer workflow output was uploaded to GitHub in SARIF format. See here: https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/uploading-a-sarif-file-to-github

scan-build can output SARIF format files simply by adding -sarif to the scan-build command in the Ledger SDK Makefile.rules_generic file

Something similar to this may also need to be added to the workflow file:

permissions:
  actions: read
  contents: read
  security-events: write
  statuses: write
.
.
.
- name: Upload scan result
uses: github/codeql-action/upload-sarif@v2
if: failure()
with:
  sarif_file: output-scan-build

[xchapron-ledger]

Hello, I did it a try and build an app with on purpose scan-build issue. The output SARIF files all looked unusable:

{
  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
  "runs": [
    {
      "artifacts": [],
      "columnKind": "unicodeCodePoints",
      "results": [],
      "tool": {
        "driver": {
          "fullName": "clang static analyzer",
          "language": "en-US",
          "name": "clang",
          "rules": [],
          "version": "Ubuntu clang version 12.0.1-19ubuntu3"
        }
      }
    }
  ],
  "version": "2.1.0"
}

While there was some actual errors:

scan-build --use-cc=clang -analyze-headers -enable-checker security -enable-checker unix -enable-checker valist -o scan-build --status-bugs -sarif make default ENABLE_SDK_WERROR=1
scan-build: Using '/usr/lib/llvm-12/bin/clang' for static analysis
Prepare directories
[GLYPH] Compiling...
[CC]   build/nanos2/obj/address.o
[CC]   build/nanos2/obj/app_main.o
[CC]   build/nanos2/obj/bagl.o
[CC]   build/nanos2/obj/bagl_animate.o
[CC]   build/nanos2/obj/bagl_display.o
[CC]   build/nanos2/obj/bagl_fonts.o
[CC]   build/nanos2/obj/bagl_glyphs.o
[CC]   build/nanos2/obj/base58.o
[CC]   build/nanos2/obj/bip32.o
[CC]   build/nanos2/obj/buffer.o
[CC]   build/nanos2/obj/checks.o
[CC]   build/nanos2/obj/crypto_helpers.o
[AS]   build/nanos2/obj/cx_stubs.o
[CC]   build/nanos2/obj/deserialize.o
[CC]   build/nanos2/obj/dispatcher.o
[CC]   build/nanos2/obj/format.o
[CC]   build/nanos2/obj/get_app_name.o
[CC]   build/nanos2/obj/get_public_key.o
[CC]   build/nanos2/obj/get_version.o
[CC]   build/nanos2/obj/glyphs.o
[CC]   build/nanos2/obj/io.o
/home/xchapron/Workspace/LedgerHQ/sdk-nanosp/lib_standard_app/io.c:51:31: error: unused parameter 'channel' [-Werror,-Wunused-parameter]
WEAK uint8_t io_event(uint8_t channel) {
                              ^
/home/xchapron/Workspace/LedgerHQ/sdk-nanosp/lib_standard_app/io.c:65:9: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
        case SEPROXYHAL_TAG_DISPLAY_PROCESSED_EVENT:
        ^
/home/xchapron/Workspace/LedgerHQ/sdk-nanosp/lib_standard_app/io.c:65:9: note: insert '__attribute__((fallthrough));' to silence this warning
        case SEPROXYHAL_TAG_DISPLAY_PROCESSED_EVENT:
        ^
        __attribute__((fallthrough)); 
/home/xchapron/Workspace/LedgerHQ/sdk-nanosp/lib_standard_app/io.c:65:9: note: insert 'break;' to avoid fall-through
        case SEPROXYHAL_TAG_DISPLAY_PROCESSED_EVENT:
        ^
        break; 
2 errors generated.
make: *** [/home/xchapron/Workspace/LedgerHQ/sdk-nanosp/Makefile.rules_generic:81: build/nanos2/obj/io.o] Error 1
scan-build: Analysis run complete.
scan-build: Analysis results (sarif files) deposited in '/home/xchapron/Workspace/LedgerHQ/app-boilerplate/scan-build/2023-05-15-174932-369686-1'

Did I missed something?

[aido]

Hi @xchapron-ledger,

I added -sarif to SDK's Makefile.rules_generic:

bash-5.1# grep "(SCAN_BUILD)" /opt/nanos-secure-sdk/Makefile.rules_generic
        $(SCAN_BUILD) --use-cc=$(CC) -analyze-headers -enable-checker security -enable-checker unix -enable-checker valist --status-bugs -sarif -o $(SCAN_BUILD_OUTPUT) $(MAKE)

I added a deliberate divide-by-zero error into some code:

// deliberate error
int test(int x){
  return 10 / (x - x); // warn
}

and ran make scan-build:

bash-5.1# make scan-build
TARGET_NAME=TARGET_NANOS TARGET_ID=0x31100004 TARGET_VERSION=2.1.0
.
.
.
scan-build --use-cc=clang -analyze-headers -enable-checker security -enable-checker unix -enable-checker valist --status-bugs -sarif -o /app/output-scan-build make
scan-build: Using '/usr/bin/clang-12' for static analysis
.
.
.
[CC]   build/nanos/obj/main.o
src/main.c:63:13: warning: Division by zero [core.DivideZero]
  return 10 / (x - x); // warn
         ~~~^~~~~~~~~
1 warning generated.
.
.
.
scan-build: Analysis run complete.
scan-build: Analysis results (sarif files) deposited in '/app/output-scan-build/2023-05-15-170319-85595-1'

scan-build seems to create a report-xxxxx.sarif file for every file it analyses. This could be quite a lot of files:

bash-5.1# ls -la /app/output-scan-build/2023-05-15-170319-85595-1/report-*.sarif | wc -l
52

Most files had no errors so had the same content similar to yours above. However, one file was different to the others, this contained info on the scan-build warning:

bash-5.1# cat /app/output-scan-build/2023-05-15-170319-85595-1/report-mt3tLQ.sarif
{
  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
  "runs": [
    {
      "artifacts": [
        {
          "length": 4912,
          "location": {
            "uri": "file:///app/src/main.c"
          },
          "mimeType": "text/plain",
          "roles": [
            "resultFile"
          ]
        }
      ],
.
.
.
                    {
                      "importance": "essential",
                      "location": {
                        "message": {
                          "text": "Division by zero"
                        },
                        "physicalLocation": {
                          "artifactLocation": {
                            "index": 0,
                            "uri": "file:///app/src/main.c"
                          },
                          "region": {
                            "endColumn": 13,
                            "startColumn": 13,
                            "startLine": 63
                          }
.
.
.

I think the upload-sarif action is smart enough to only upload sarif files that have details of warnings and errors in them,