Calling SbomValidator.ValidateSbomAsync With Directory As OutputPath Returns False Positive

[rdgillespie]

Current Behaviour

While validating a buildDropPath where one of the files has been intentionally tampered with, passing a directory to the outputPath parameter of the SbomValidator.ValidateSbomAsync method, the returned SBOMValidationResult has the IsSuccessful property set to true.

Expected Behaviour

While validating a buildDropPath where one of the files has been intentionally tampered with, passing a directory to the outputPath parameter of the SbomValidator.ValidateSbomAsync method, the returned SBOMValidationResult has the IsSuccessful property set to false.

Alternatively, an exception should be thrown if the output file cannot be written to.

Steps to Reproduce

1. Created an artifact and generate an SBOM
2. Tamper with one of the files so that hash changes
3. Call ISBOMValidator.ValidateSbomAsync with the outputPath parameter as an existing directory path
4. Inspect returned result

Additonal Context

Library version used: 3.1.0

[rdgillespie]

I've confirmed that this is reproducible on the latest package.

[JoseRenan]

Hi, thanks for reporting, would you mind giving more details on how are you running? Like, which kind of project you're generating the SBOM for? (python? c#? java?), or which change on which type of file you're trying to do to get the validation to succeed after the change?

What I tried:

• Created a simple folder with a main.py fille, consisting of only a print("Hello World!")
• Ran the code with the API to generate the SBOM, using the library on version 3.1.0
• Edited the main.py file to now have print("Hello World!!!")
• Ran the code with the API to validate the same SBOM and print the result.IsSuccess, it printed false and the JSON output also shows:

 {Result=Failure, Errors=ErrorContainer`1 {Count=1, Errors=[FileValidationResult {Path="./main.py", ErrorType=InvalidHash}]}

Let me know if I followed the right steps

[rdgillespie]

We're using the C# lib to validate the content of a folder. Since we are using the lib, we're depending on the SbomValidationResult returned by ISBOMValidator.ValidateSbomAsync to determine the success/failure of the validation. Listening to logs for results would be a pain.

Here is a trimmed down C# example of the behaviour we're seeing. I've written it with the new app.cs format, but it repos exactly the same in .Net8.

#:package Microsoft.Sbom.Api@4.0.3
#:package Microsoft.Sbom.Extensions.DependencyInjection@4.0.3

using Microsoft.Sbom.Contracts;
using Microsoft.Sbom.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

async Task RunSbomValidationAsync(string artifactPath, string outputPath)
{
    ServiceCollection services = new ServiceCollection();

    // Trim out the logging to clean up output for example
    services.AddSbomTool(Serilog.Events.LogEventLevel.Fatal);

    var serviceProvider = services.BuildServiceProvider();
    var sbomTool = serviceProvider.GetRequiredService<ISbomValidator>();

    var results = await sbomTool.ValidateSbomAsync(
                        artifactPath,
                        outputPath,
                        new[] { new SbomSpecification("SPDX", "2.2") },
                        ignoreMissing: false);

    Console.WriteLine($"IsSuccess: {results.IsSuccess}");
    Console.WriteLine("Errors:");
    foreach (var error in results.Errors)
    {
        Console.WriteLine($"- {error}");
    }
}


// Create some extra content to tamper with the artifact
using (var writer = new StreamWriter("C:\\temp\\cv\\some-unknown-file", true))
{
    writer.WriteLine("// This is a test file for SBOM validation.");
    writer.WriteLine("// It contains some random text at the end.");
}

Console.WriteLine("Starting SBOM validation with outputPath pointing to a directory...");
await RunSbomValidationAsync("C:\\temp\\cv", "C:\\temp\\");

Console.WriteLine();
Console.WriteLine("============================================================");
Console.WriteLine();

Console.WriteLine("Starting SBOM validation with outputPath pointing to a file...");
await RunSbomValidationAsync("C:\\temp\\cv", "C:\\temp\\results.json");

Output:

Starting SBOM validation with outputPath pointing to a directory...
IsSuccess: True
Errors:

============================================================

Starting SBOM validation with outputPath pointing to a file...

{ ... Omitted for brevity ... }

IsSuccess: False
Errors:
- Microsoft.Sbom.Contracts.EntityError
- Microsoft.Sbom.Contracts.EntityError

As we can see with this example, if a folder is passed as an output path, the SbomValidationResult is successful with no errors. If the same check is execute with a file passed as an output path the SbomValidationResult indicates a failure with errors. The tool does log failures or success to the Sirilogger, but not to the object that is returned be the ISbomValidator.

[JoseRenan]

Hey @rdgillespie, yeah, it makes sense. I was able to replicate the issue. This is happening because in this line:

sbom-tool/src/Microsoft.Sbom.Api/SbomValidator.cs

Line 107 in 7674e09

return new SbomValidationResult(!errors.Any(), errors);

We are setting the validation result only based on the list of validation errors, but what happens when you set an outputPath that is a dir instead of file is an exception, not a validation error, and that is not listed on the errors list, but on the exceptions list. I will mark the issue to be triaged and prioritized by the team!

[JoseRenan]

Hey @rdgillespie, yeah, it makes sense. I was able to replicate the issue. This is happening because in this line:

sbom-tool/src/Microsoft.Sbom.Api/SbomValidator.cs

Line 107 in 7674e09

return new SbomValidationResult(!errors.Any(), errors);
We are setting the validation result only based on the list of validation errors, but what happens when you set an outputPath that is a dir instead of file is an exception, not a validation error, and that is not listed on the errors list, but on the exceptions list. I will mark the issue to be triaged and prioritized by the team!

A workaround for this in the meanwhile would be checking the generated output file, instead of checking the variable or the logs, if that is feasible 🤔