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
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add registrypreview fuzzing code
  • Loading branch information
chenmy77 committed Feb 24, 2025
commit 330e2920374d19247cd2715a3a4805489348a923
2 changes: 1 addition & 1 deletion .pipelines/v2/templates/job-fuzz.yml
Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@ jobs:
displayName: Download artifacts
artifact: $(ArtifactName)
patterns: |-
**/tests/*.FuzzTests/**
**/tests/RegistryPreview.FuzzTests/**

- task: onefuzz-task@0
inputs:
159 changes: 149 additions & 10 deletions src/modules/registrypreview/RegistryPreview.FuzzTests/FuzzTests.cs
Original file line number Diff line number Diff line change
@@ -2,26 +2,165 @@
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using System;
using System.Diagnostics;
using System.Globalization;
using System.Resources;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Microsoft.Win32;
using RegistryPreviewUILib;

namespace RegistryPreview.FuzzTests
{
public class FuzzTests
{
public static void FuzzParseRegistryFile(ReadOnlySpan<byte> input)
private const string REGISTRYHEADER4 = "regedit4";
private const string REGISTRYHEADER5 = "windows registry editor version 5.00";
private const string KEYIMAGE = "ms-appx:///Assets/RegistryPreview/folder32.png";
private const string DELETEDKEYIMAGE = "ms-appx:///Assets/RegistryPreview/deleted-folder32.png";

public static void FuzzCheckKeyLineForBrackets(ReadOnlySpan<byte> input)
{
try
string registryLine;

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.


string[] registryLines = filenameText.Split("\r");

if (registryLines.Length <= 1)
{
return;
}

// REG files have to start with one of two headers and it's case-insensitive
registryLine = registryLines[0];

if (!IsValidRegistryHeader(registryLine))
{
return;
}

int index = 1;
registryLine = registryLines[index];

ParseHelper.ProcessRegistryLine(registryLine);
if (registryLine.StartsWith("[-", StringComparison.InvariantCulture))
{
// remove the - as we won't need it but it will get special treatment in the UI
registryLine = registryLine.Remove(1, 1);

string imageName = DELETEDKEYIMAGE;
ParseHelper.CheckKeyLineForBrackets(ref registryLine, ref imageName);
}
else if (registryLine.StartsWith('['))
{
string imageName = KEYIMAGE;
ParseHelper.CheckKeyLineForBrackets(ref registryLine, ref imageName);
}
else
{
return;
}
}

public static void FuzzStripFirstAndLast(ReadOnlySpan<byte> input)
{
string registryLine;

var filenameText = GenerateRegistryHeader(input);
Console.WriteLine($"\n input1 is {filenameText}");

filenameText = filenameText.Replace("\r\n", "\r");
string[] registryLines = filenameText.Split("\r");

if (registryLines.Length <= 1)
{
return;
}

// REG files have to start with one of two headers and it's case-insensitive
registryLine = registryLines[0];

if (!IsValidRegistryHeader(registryLine))
{
var text = System.Text.Encoding.UTF8.GetString(input);

return;
}
catch (Exception ex) when (ex is ArgumentException)

int index = 1;
registryLine = registryLines[index];
Console.WriteLine($"\n registryLine1 is {registryLine}");

ParseHelper.ProcessRegistryLine(registryLine);

if (registryLine.StartsWith("[-", StringComparison.InvariantCulture))
{
// remove the - as we won't need it but it will get special treatment in the UI
registryLine = registryLine.Remove(1, 1);

string imageName = DELETEDKEYIMAGE;
ParseHelper.CheckKeyLineForBrackets(ref registryLine, ref imageName);
Console.WriteLine($"\n CheckKeyLineForBrackets is {registryLine}");

registryLine = ParseHelper.StripFirstAndLast(registryLine);
}
else if (registryLine.StartsWith('['))
{
string imageName = KEYIMAGE;
ParseHelper.CheckKeyLineForBrackets(ref registryLine, ref imageName);
Console.WriteLine($"\n CheckKeyLineForBrackets2 is {registryLine}");

registryLine = ParseHelper.StripFirstAndLast(registryLine);
}
else if (registryLine.StartsWith('"') && registryLine.EndsWith("=-", StringComparison.InvariantCulture))
{
registryLine = registryLine.Replace("=-", string.Empty);

// remove the "'s without removing all of them
registryLine = ParseHelper.StripFirstAndLast(registryLine);
}
else if (registryLine.StartsWith('"'))
{
int equal = registryLine.IndexOf('=');
if ((equal < 0) || (equal > registryLine.Length - 1))
{
// something is very wrong
return;
}

// set the name and the value
string name = registryLine.Substring(0, equal);

// trim the whitespace and quotes from the name
name = name.Trim();
name = ParseHelper.StripFirstAndLast(name);
}
else
{
return;
}
}

public static string GenerateRegistryHeader(ReadOnlySpan<byte> input)
{
string header = new Random().Next(2) == 0 ? REGISTRYHEADER4 : REGISTRYHEADER5;

string inputText = System.Text.Encoding.UTF8.GetString(input);
string filenameText = header + "\r\n" + inputText;

return filenameText.Replace("\r\n", "\r");
}

private static bool IsValidRegistryHeader(string line)
{
// 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.

{
// This is an example. It's important to filter out any *expected* exceptions from our code here.
// However, catching all exceptions is considered an anti-pattern because it may suppress legitimate
// issues, such as a NullReferenceException thrown by our code. In this case, we still re-throw
// the exception, as the ToJsonFromXmlOrCsvAsync method is not expected to throw any exceptions.
throw;
case REGISTRYHEADER4:
case REGISTRYHEADER5:
return true;
default:
return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@
"$type": "libfuzzerDotNet",
"dll": "RegistryPreview.FuzzTests.dll",
"class": "RegistryPreview.FuzzTests.FuzzTests",
"method": "FuzzTargetMethod",
"method": "FuzzCheckKeyLineForBrackets",
"FuzzingTargetBinaries": [
"PowerToys.RegistryPreview.dll"
]
@@ -30,7 +30,48 @@
// at least one job is required
{
"projectName": "RegistryPreview",
"targetName": "RegistryPreview-dotnet-fuzzer"
"targetName": "RegistryPreview-dotnet-CheckKeyLineForBrackets-fuzzer"
}
],
"jobDependencies": [
// this should contain, at minimum,
// the DLL and PDB files
// you will need to add any other files required
// (globs are supported)
"RegistryPreview.FuzzTests.dll",
"RegistryPreview.FuzzTests.pdb",
"Microsoft.Windows.SDK.NET.dll",
"WinRT.Runtime.dll"
],
"SdlWorkItemId": 49911822
},
{
"fuzzer": {
"$type": "libfuzzerDotNet",
"dll": "RegistryPreview.FuzzTests.dll",
"class": "RegistryPreview.FuzzTests.FuzzTests",
"method": "FuzzStripFirstAndLast",
"FuzzingTargetBinaries": [
"PowerToys.RegistryPreview.dll"
]
},
"adoTemplate": {
// supply the values appropriate to your
// project, where bugs will be filed
"org": "microsoft",
"project": "OS",
"AssignedTo": "mengyuanchen@microsoft.com",
"AreaPath": "OS\\Windows Client and Services\\WinPD\\DEEP-Developer Experience, Ecosystem and Partnerships\\SHINE\\PowerToys",
"IterationPath": "OS\\Future"
},
"jobNotificationEmail": "mengyuanchen@microsoft.com",
"skip": false,
"rebootAfterSetup": false,
"oneFuzzJobs": [
// at least one job is required
{
"projectName": "RegistryPreview",
"targetName": "RegistryPreview-dotnet-StripFirstAndLasts-fuzzer"
}
],
"jobDependencies": [
@@ -41,7 +82,6 @@
"RegistryPreview.FuzzTests.dll",
"RegistryPreview.FuzzTests.pdb",
"Microsoft.Windows.SDK.NET.dll",
"Newtonsoft.Json.dll",
"WinRT.Runtime.dll"
],
"SdlWorkItemId": 49911822
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<TargetFramework>net7.0</TargetFramework>
<LangVersion>latest</LangVersion>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
@@ -11,6 +11,10 @@
<OutputPath>..\..\..\..\$(Platform)\$(Configuration)\tests\RegistryPreview.FuzzTests\</OutputPath>
</PropertyGroup>

<ItemGroup>
<Compile Include="..\RegistryPreviewUILib\ParseHelper.cs" Link="ParseHelper.cs" />
</ItemGroup>

<ItemGroup>
<Content Include="OneFuzzConfig.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
112 changes: 112 additions & 0 deletions src/modules/registrypreview/RegistryPreviewUILib/ParseHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace RegistryPreviewUILib
{
public class ParseHelper
{
private const string ERRORIMAGE = "ms-appx:///Assets/RegistryPreview/error32.png";

/// <summary>
/// Checks a Key line for the closing bracket and treat it as an error if it cannot be found
/// </summary>
public static void CheckKeyLineForBrackets(ref string registryLine, ref string imageName)
{
// following the current behavior of the registry editor, find the last ] and treat everything else as ignorable
int lastBracket = registryLine.LastIndexOf(']');
if (lastBracket == -1)
{
// since we don't have a last bracket yet, add an extra space and continue processing
registryLine += " ";
imageName = ERRORIMAGE;
}
else
{
// having found the last ] and there is text after it, drop the rest of the string on the floor
if (lastBracket < registryLine.Length - 1)
{
registryLine = registryLine.Substring(0, lastBracket + 1);
}

if (CheckForKnownGoodBranches(registryLine) == false)
{
imageName = ERRORIMAGE;
}
}
}

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

{
if ((key.StartsWith("[HKEY_CLASSES_ROOT]", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith("[HKEY_CURRENT_USER]", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith("[HKEY_USERS]", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith("[HKEY_LOCAL_MACHINE]", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith("[HKEY_CURRENT_CONFIG]", StringComparison.InvariantCultureIgnoreCase) == false)
&&
(key.StartsWith(@"[HKEY_CLASSES_ROOT\", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith(@"[HKEY_CURRENT_USER\", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith(@"[HKEY_USERS\", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith(@"[HKEY_LOCAL_MACHINE\", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith(@"[HKEY_CURRENT_CONFIG\", StringComparison.InvariantCultureIgnoreCase) == false)
&&
(key.StartsWith("[HKCR]", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith("[HKCU]", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith("[HKU]", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith("[HKLM]", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith("[HKCC]", StringComparison.InvariantCultureIgnoreCase) == false)
&&
(key.StartsWith(@"[HKCR\", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith(@"[HKCU\", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith(@"[HKU\", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith(@"[HKLM\", StringComparison.InvariantCultureIgnoreCase) == false &&
key.StartsWith(@"[HKCC\", StringComparison.InvariantCultureIgnoreCase) == false))
{
return false;
}

return true;
}

/// <summary>
/// Rip the first and last character off a string,
/// checking that the string is at least 2 characters long to avoid errors
/// </summary>
public static string StripFirstAndLast(string line)
{
if (line.Length > 1)
{
line = line.Remove(line.Length - 1, 1);
line = line.Remove(0, 1);
}

return line;
}

public static string ProcessRegistryLine(string registryLine)
{
if (registryLine.StartsWith("@=-", StringComparison.InvariantCulture))
{
// REG file has a callout to delete the @ Value which won't work *but* the Registry Editor will
// clear the value of the @ Value instead, so it's still a valid line.
registryLine = registryLine.Replace("@=-", "\"(Default)\"=\"\"");
}
else if (registryLine.StartsWith("@=", StringComparison.InvariantCulture))
{
// This is the Value called "(Default)" so we tweak the line for the UX
registryLine = registryLine.Replace("@=", "\"(Default)\"=");
}

return registryLine;
}
}
}
Loading
Oops, something went wrong.