Skip to content

Fix <img /> detection regex in web cmdlets #12099

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.

Sign up for GitHub

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

Merged
anmenaga merged 5 commits into from
Mar 24, 2020
Merged

Fix <img /> detection regex in web cmdlets #12099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3971046
:bug: Fix img detection regex.
vexx32 Mar 11, 2020
4c25150
:white_check_mark: Add test case
vexx32 Mar 17, 2020
bb42b21
:memo: addressed review comments
vexx32 Mar 21, 2020
daec401
:recycle: move test to new context
vexx32 Mar 21, 2020
1792218
Merge branch 'master' into Web/ImageRegex
TravisEz13 Mar 23, 2020
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
2 changes: 1 addition & 1 deletion ...l.Commands.Utility/commands/utility/WebCmdlet/Common/BasicHtmlWebResponseObject.Common.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ private void EnsureHtmlParser()

if (s_imageRegex == null)
{
s_imageRegex = new Regex(@"<img\s+[^\s>]*>",
s_imageRegex = new Regex(@"<img\s[^>]*?>",
RegexOptions.Singleline | RegexOptions.IgnoreCase | RegexOptions.Compiled);
}
}
Expand Down
44 changes: 44 additions & 0 deletions test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1900,7 +1900,51 @@ Describe "Invoke-WebRequest tests" -Tags "Feature", "RequireAdminOnWindows" {
}
}

Context "Regex Parsing" {

It 'correctly parses an image with id, class, and src attributes' {
$dosUri = Get-WebListenerUrl -Test 'Dos' -query @{
dosType = 'img-attribute'
}

$response = Invoke-WebRequest -Uri $dosUri
$response.Images | Should -Not -BeNullOrEmpty
}
}

Context "Denial of service" -Tag 'DOS' {
It "Image Parsing" {
Set-ItResult -Pending -Because "The pathological regex runs fast due to https://github.com/dotnet/runtime/issues/33399. Fixed in .NET 5 preview.2"
$dosUri = Get-WebListenerUrl -Test 'Dos' -query @{
dosType='img'
dosLength='5000'
}
$script:content = ''
[TimeSpan] $timeSpan = Measure-Command {
$response = Invoke-WebRequest -Uri $dosUri
$script:content = $response.content
$response.Images | out-null
}

$script:content | Should -Not -BeNullOrEmpty

# pathological regex
$regex = [RegEx]::new('<img\s+[^>]*>')

[TimeSpan] $pathologicalTimeSpan = Measure-Command {
$regex.Match($content)
}

$pathologicalRatio = $pathologicalTimeSpan.TotalMilliseconds/$timeSpan.TotalMilliseconds
Write-Verbose "Pathological ratio: $pathologicalRatio" -Verbose

# dosLength 4,000 on my 3.5 GHz 6-Core Intel Xeon E5 macpro produced a ratio of 12
# dosLength 5,000 on my 3.5 GHz 6-Core Intel Xeon E5 macpro produced a ratio of 21
# dosLength 10,000 on my 3.5 GHz 6-Core Intel Xeon E5 macpro produced a ratio of 75
# in some cases we will be running in a Docker container with modest resources
$pathologicalRatio | Should -BeGreaterThan 5
}

It "Charset Parsing" {
$dosUri = Get-WebListenerUrl -Test 'Dos' -query @{
dosType='charset'
Expand Down
17 changes: 11 additions & 6 deletions test/tools/WebListener/Controllers/DosController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,33 +29,38 @@ public string Index()
}

StringValues dosLengths;
Int32 dosLength =1;
Int32 dosLength = 1;
if (Request.Query.TryGetValue("dosLength", out dosLengths))
{
Int32.TryParse(dosLengths.FirstOrDefault(), out dosLength);
}

string body = string.Empty;
switch(dosType)
switch (dosType)
{
case "img":
contentType = "text/html; charset=utf8";
body = "<img" + (new string(' ', dosLength));
break;
// This is not really a DOS test, but this is the best place for it at present.
case "img-attribute":
Copy link
Member

@TravisEz13 TravisEz13 Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't actually a DOS... can you just add a comment explaining that?

Copy link
Collaborator Author

@vexx32 vexx32 Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TravisEz13 Yeah I was torn on that myself. I could also just add a new Controller class here instead, since we plan on adding future cases to test the actual regex patterns' capability to parse what we expect?

Copy link
Member

@TravisEz13 TravisEz13 Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to. I'll leave it up to you, but at least add a comment that this is not a DOS case, but you didn't want to create a new controller for a case that was so similar.

contentType = "text/html; charset=utf8";
body = "<img src=\"https://fakesite.org/image.png\" id=\"mainImage\" class=\"lightbox\">";
break;
case "charset":
contentType = "text/html; charset=melon";
body = "<meta " + (new string('.', dosLength));
break;
default:
throw new InvalidOperationException("Invalid dosType: "+dosType);
throw new InvalidOperationException("Invalid dosType: " + dosType);
}

// Content-Type must be applied right before it is sent to the client or MVC will overwrite.
Response.OnStarting(state =>
{
var httpContext = (HttpContext) state;
httpContext.Response.ContentType = contentType;
return Task.FromResult(0);
var httpContext = (HttpContext)state;
httpContext.Response.ContentType = contentType;
return Task.FromResult(0);
}, HttpContext);

Response.ContentLength = Encoding.UTF8.GetBytes(body).Length;
Expand Down