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

[Fuzz] Add Fuzz testing for RegistryPreview #37607

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

chenmy77
Copy link
Contributor

@chenmy77 chenmy77 commented Feb 24, 2025

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:

  • Fuzz Tests Project:

Created for RegisrtyPreview using .NET 8 (Windows), focusing on testing CheckKeyLineForBrackets and StripFirstAndLast.

  • Code changes:

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.

  • Linking & Dependency Management:

Connected the ParseHelper.cs file to the FuzzTests project.

  • Testing Result:

The code is being tested using the OneFuzz pipeline.

  • The fuzz job result:

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

StripFirstAndLast
https://onefuzz-ui.microsoft.com/jobs/ac805e56-28b4-45ca-b7bb-f757dab6303e
image

PR Checklist

  • Closes: #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@chenmy77 chenmy77 changed the title Dev/mengyuanchen/add onefuzz/registrypreview [Fuzz] Add Fuzz testing for RegistryPreview Feb 24, 2025
@chenmy77 chenmy77 marked this pull request as ready for review February 25, 2025 09:23
// Convert the line to lowercase once for comparison
var lineLower = line.ToLowerInvariant();

switch (line)
Copy link
Contributor

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?

Copy link
Contributor Author

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);
Copy link
Contributor

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?

Copy link
Contributor Author

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.

/// <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)

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)); }

Copy link
Contributor Author

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.

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.

4 participants