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

Fix for empty Ref Exception #1

Merged

Conversation

dahlsailrunner
Copy link
Contributor

This is a proposed fix for the empty reference exception. Let me know if there are other changes necessary or if something further upstream should be changed rather than this. Hopefully this helps.

@digitalcoyote
Copy link
Owner

Thank you, I'll take a look at this as soon as I get a chance, but off-hand nothing looks amiss with your PR.

@dahlsailrunner
Copy link
Contributor Author

Ok cool. Another couple of questions, though, and I can add some documentation into the readme if you give me some answers:

  • Which NuGetDefense.json gets used when a build is triggered inside a VS project with NuGetDefense? Seems like one gets created alongide solution file and then another inside project directory next to the csproj file. (I'm trying to add settings to make sure NVD is self-updated, and haven't seen any effect - also trying to Ignore Bootstrap as an experiment to no effect either)
  • How do you ignore a package generically? I added Bootstrap with a null version and null package url and that didn't seem to have any effect - wondering if I'm doing something wrong.
  • There may be a bug in the package version evaluation for Bootstrap. I had version 3.4.1 installed which triggered some vulnerability issues (correct) but then when I updated the version to 4.4.1 the same vulnerabilities persisted during new builds (seems incorrect). If you agree I can try some experiments to troubleshoot.
  • Is is true that only ONE of the OSSIndex or NVD options should be enabled? NVD wins if both are - since the vulnDict dictionary is overwritten by the NVD branch in Program.cs. Is that correct / intentional?

@digitalcoyote
Copy link
Owner

  • Which NuGetDefense.json gets used when a build is triggered inside a VS project with NuGetDefense? It should only be creating a NuGetDefense.json per project file (in the same directory with that project). It uses $(MSBuildProjectFullPath) for msbuild to tell it what directory to look at for the config (using Path.Combine(directory, "NuGetDefense.json") )
  • How do you ignore a package generically? I'll need to doublecheck, but it should allow that by just listing the name and leaving the rest null (packageurl is readonly anyway).
  • There may be a bug in the package version evaluation for Bootstrap. I was not able to reproduce this. the version resolutiojn is handled by NuGet/OSSIndex.
  • Is is true that only ONE of the OSSIndex or NVD options should be enabled? It passes in the vulnDict when NVD overrides it. The logic within NVD will add to it if the cve is not found in vulnDict.

I'll do some tests to verify that everything is behaving as it should.

@dahlsailrunner
Copy link
Contributor Author

Awesome info. Let's get my issue resolved, I'll do some tests and assuming everything works out ok, I'll submit a new PR for some readme updates. Thanks a lot for the help / response.

Tests reporting when references is null.
Added a space to VulnerabilityReports.cs for consistency.
@digitalcoyote digitalcoyote merged commit 23c6f92 into digitalcoyote:master Apr 20, 2020
@digitalcoyote
Copy link
Owner

I added a unit test for this specific case and merged.

@digitalcoyote digitalcoyote changed the title proposed fix for empty ref exception Fix for empty Ref Exception Apr 20, 2020
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

Successfully merging this pull request may close these issues.

2 participants