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

When an attribute is immediately preceded by "/", first character is cut off from the attribute's name #16

Closed
robhol opened this issue Jun 13, 2017 · 7 comments · Fixed by #17
Assignees

Comments

@robhol
Copy link

robhol commented Jun 13, 2017

So this is one of those things that web browsers decided to let people get away with for unknown reasons:

<img src="x"/onerror="alert('onerror1')">
<img/src="x"/onerror="alert('onerror2')">

This will actually parse in browsers (tested in Chrome and FF), and can be used to inject scripts where they don't belong.

Problem

When an attribute is immediately preceded by "/", the first character is cut off from the attribute's name.

Demonstration

This minimal RoslynPad snippet illustrates the problem:

#r "$NuGet\HtmlAgilityPack\1.5.0-beta6\lib\Net45\HtmlAgilityPack.dll"
using HtmlAgilityPack;

var html = "<img src=\"x\"/onerror=\"alert('onerror1')\"><img/src=\"x\"/onerror=\"alert('onerror2')\">";
var doc = new HtmlDocument();
doc.LoadHtml(html);

doc.DocumentNode.Descendants().SelectMany(x => x.Attributes).Select(x => x.Name).Dump();

Expected: src, onerror, src, onerror - Got: src, nerror, rc, nerror

Affected versions

  • 1.5.0-beta6
  • 1.4.9.5
@JonathanMagnan
Copy link
Member

Hello @robhol ,

Thank you for reporting this issue. If the time permits, we will try to verify and merge the pull request made by @josteink today or tomorrow.

More information will be provided soon.

Best Regards,

Jonathan

@JonathanMagnan
Copy link
Member

JonathanMagnan commented Jun 13, 2017

Hello @robhol ,

We merged the pull request: #17

In the v1.5.0-beta7
https://www.nuget.org/packages/HtmlAgilityPack/

Let us know if the issue is fixed on your side.

Best Regards,

Jonathan

@robhol
Copy link
Author

robhol commented Jun 15, 2017

I can confirm that the snippet now behaves as expected in beta8 - unfortunately I discovered another bug as well. It's closely related, so I'll post it here for now and can post a separate one if necessary.

This snippet:
<img src="x"//onerror="alert('onerror1')">

is now interpreted by HAP as having the attribute "/onerror", but the script in it will run in Chrome, Firefox and Edge.

@JonathanMagnan
Copy link
Member

Hello @robhol ,

You are right. We will try to investigate and fix this issue this weekend.

Best Regards,

Jonathan

@JonathanMagnan
Copy link
Member

JonathanMagnan commented Jun 17, 2017

Hello @robhol ,

The fix has been completed. It will be released within a few hours.

The code should not support many "Empty Tag"

Best Regards,

Jonathan

@JonathanMagnan
Copy link
Member

Hello @robhol ,

The version v1.5.0-beta9 has been released:
https://www.nuget.org/packages/HtmlAgilityPack/1.5.0-beta9

Let us know if the issue is fixed.

Best Regards,

Jonathan

@JonathanMagnan
Copy link
Member

Closing Comment: Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants