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

AnalyzeCommand base class should handle obsolete ComputeFileHashes arg #2211

Closed
michaelcfanning opened this issue Jan 3, 2021 · 2 comments · Fixed by #2231
Closed

AnalyzeCommand base class should handle obsolete ComputeFileHashes arg #2211

michaelcfanning opened this issue Jan 3, 2021 · 2 comments · Fixed by #2231
Assignees

Comments

@michaelcfanning
Copy link
Member

michaelcfanning commented Jan 3, 2021

This argument is obsolete in the driver framework, replaced by --insert Hashes. We've marked it as obsolete and it warns on compilation dependency. But I just updated BinSkim to the latest SDK and the driver framework isn't providing backwards compatibility for use of the obsolete arg.

I've added the following code to BinSkim but really it should be in AnalyzeCommand.Run:

#pragma warning disable CS0618 // Type or member is obsolete
            if (analyzeOptions.ComputeFileHashes)
#pragma warning restore CS0618
            {
                OptionallyEmittedData dataToInsert = analyzeOptions.DataToInsert.ToFlags();
                dataToInsert |= OptionallyEmittedData.Hashes;

                analyzeOptions.DataToInsert  = 
                    dataToInsert.ToString().Split('|')
                        .Select(d => { return Enum.Parse<OptionallyEmittedData>(d); })
                        .ToList();
@michaelcfanning
Copy link
Member Author

The framework should also log a notification about the use of the obsolete command, arguable. Minimally, this notification should surface as a warning (and we can elevate to an error in the future to enforce upgrade).

@michaelcfanning
Copy link
Member Author

@eddynaka, good first issue. Maybe for James when he arrives?

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 a pull request may close this issue.

2 participants