-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
[Fuzz] Add Fuzz testing for RegistryPreview #37607
base: main
Are you sure you want to change the base?
Conversation
…onefuzz/registrypreview
src/modules/registrypreview/RegistryPreview.FuzzTests/RegistryPreview.FuzzTests.csproj
Show resolved
Hide resolved
// Convert the line to lowercase once for comparison | ||
var lineLower = line.ToLowerInvariant(); | ||
|
||
switch (line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that you're using switch (line), which relies on the original line, but you convert lineLower to lowercase for comparison. Which one do you actually intend to use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intend to use the line, and the lineLower needs to be removed.
string registryLine; | ||
|
||
// Simulate registry file content as filenameText | ||
var filenameText = GenerateRegistryHeader(input); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the comments, the result represents registry file content, so the name filenameText might be confusing. Would registryContent be a better choice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I also think so,registryContent would be a better choice than filenameText. I will modify it.
This comment has been minimized.
This comment has been minimized.
This reverts commit e8269b8.
/// <summary> | ||
/// Make sure the root of a full path start with one of the five "hard coded" roots. Throw an error for the branch if it doesn't. | ||
/// </summary> | ||
private static bool CheckForKnownGoodBranches(string key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can it be simplified like this?
private static bool CheckForKnownGoodBranches(string key) { string[] knownGoodPrefixes = new[] { "[HKEY_CLASSES_ROOT]", "[HKEY_CURRENT_USER]", "[HKEY_USERS]", "[HKEY_LOCAL_MACHINE]", "[HKEY_CURRENT_CONFIG]", @"[HKEY_CLASSES_ROOT\", @"[HKEY_CURRENT_USER\", @"[HKEY_USERS\", @"[HKEY_LOCAL_MACHINE\", @"[HKEY_CURRENT_CONFIG\", "[HKCR]", "[HKCU]", "[HKU]", "[HKLM]", "[HKCC]", @"[HKCR\", @"[HKCU\", @"[HKU\", @"[HKLM\", @"[HKCC\" }; return knownGoodPrefixes.Any(prefix => key.StartsWith(prefix, StringComparison.InvariantCultureIgnoreCase)); }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right. However, it's the original code in RegistryPreviewMainpage.Utilities.cs, so I don’t intend to modify it.
Summary of the Pull Request
Why we need add fuzz test?
This is a request from the SFI Wave 3, currently set as Priority 2.
In this PR:
Created for RegisrtyPreview using .NET 8 (Windows), focusing on testing CheckKeyLineForBrackets and StripFirstAndLast.
To facilitate fuzz testing of the registry file parsing process, decouple the utility functions CheckKeyLineForBrackets and StripFirstAndLast from the ParseRegistryFile function in RegistryPreviewMainPage.Utilities.cs. Move them to a new ParseHelper.cs and change their access modifiers from private to public.
Added the RegistryPreview.FuzzTests project to RegistryPreview.
Implemented CheckKeyLineForBrackets and FuzzStripFirstAndLast in the RegistryPreview.FuzzTests project.
Connected the ParseHelper.cs file to the FuzzTests project.
The code is being tested using the OneFuzz pipeline.
CheckKeyLineForBrackets

https://onefuzz-ui.microsoft.com/jobs/0e9d3878-3a76-4a73-b2d1-5d5a8b87ace3
StripFirstAndLast

https://onefuzz-ui.microsoft.com/jobs/ac805e56-28b4-45ca-b7bb-f757dab6303e
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed