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

Boolean attribute getters throw if the attribute name is not all lowercase #3133

Closed
mgol opened this Issue May 29, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@mgol
Member

mgol commented May 29, 2016

Description

In #2916 we removed our logic that lowercased attribute names. This caused one regression: any attribute getter using a name for boolean attributes but not all lowercased is going into an infinite recursion, exceeding the stack call limit.

Amongst others, this is breaking the AngularJS test suite when tested with jQuery 3.0.0-rc1.

Link to test case

https://jsfiddle.net/shnann6y/2/

Basically, $('<div>').attr('requiRed') is enough to trigger the error.

@mgol mgol added this to the 3.0.1 milestone May 29, 2016

@mgol mgol self-assigned this May 29, 2016

mgol added a commit to mgol/jquery that referenced this issue May 29, 2016

Attributes: Avoid infinite recursion on non-lowercase attribute getters
Attribute hooks are determined for the lowercase versions of attribute names
but this has not been reflected in the bool attribute hooks. The code that
temporarily removed a handler to avoid an infinite loop was removing an
incorrect handler causing stack overflow.

Fixes gh-3133
Refs gh-2914
Refs gh-2916
@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol May 29, 2016

Member

PR: #3134.

Member

mgol commented May 29, 2016

PR: #3134.

@mgol mgol removed the Needs review label May 29, 2016

@mgol mgol modified the milestones: 3.0.0, 3.0.1 May 29, 2016

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol May 29, 2016

Member

We can discuss but I'm changing the milestone to 3.0.0 for now as it breaks the Angular test suite.

Member

mgol commented May 29, 2016

We can discuss but I'm changing the milestone to 3.0.0 for now as it breaks the Angular test suite.

@mgol mgol added the Needs review label May 29, 2016

mgol added a commit to mgol/jquery that referenced this issue May 29, 2016

Attributes: Avoid infinite recursion on non-lowercase attribute getters
Attribute hooks are determined for the lowercase versions of attribute names
but this has not been reflected in the bool attribute hooks. The code that
temporarily removed a handler to avoid an infinite loop was removing an
incorrect handler causing stack overflow.

Fixes gh-3133
Refs gh-2914
Refs gh-2916

mgol added a commit to mgol/jquery that referenced this issue May 29, 2016

Attributes: Avoid infinite recursion on non-lowercase attribute getters
Attribute hooks are determined for the lowercase versions of attribute names
but this has not been reflected in the bool attribute hooks. The code that
temporarily removed a handler to avoid an infinite loop was removing an
incorrect handler causing stack overflow.

Fixes gh-3133
Refs gh-2914
Refs gh-2916

mgol added a commit to mgol/jquery that referenced this issue May 31, 2016

Attributes: Avoid infinite recursion on non-lowercase attribute getters
Attribute hooks are determined for the lowercase versions of attribute names
but this has not been reflected in the bool attribute hooks. The code that
temporarily removed a handler to avoid an infinite loop was removing an
incorrect handler causing stack overflow.

Fixes gh-3133
Refs gh-2914
Refs gh-2916

@mgol mgol added Has Pull Request and removed Needs review labels Jun 2, 2016

mgol added a commit to mgol/jquery that referenced this issue Jun 3, 2016

Attributes: Avoid infinite recursion on non-lowercase attribute getters
Attribute hooks are determined for the lowercase versions of attribute names
but this has not been reflected in the bool attribute hooks. The code that
temporarily removed a handler to avoid an infinite loop was removing an
incorrect handler causing stack overflow.

Fixes gh-3133
Refs gh-2914
Refs gh-2916

mgol added a commit to mgol/jquery that referenced this issue Jun 3, 2016

Attributes: Avoid infinite recursion on non-lowercase attribute getters
Attribute hooks are determined for the lowercase versions of attribute names
but this has not been reflected in the bool attribute hooks. The code that
temporarily removed a handler to avoid an infinite loop was removing an
incorrect handler causing stack overflow.

Fixes gh-3133
Refs gh-2914
Refs gh-2916
Closes gh-3134

@mgol mgol closed this in #3134 Jun 3, 2016

olivierlacan added a commit to orientation/orientation that referenced this issue Jun 30, 2017

olivierlacan added a commit to orientation/orientation that referenced this issue Jun 30, 2017

@anarcat

This comment has been minimized.

Show comment
Hide comment
@anarcat

anarcat Jan 18, 2018

CVE-2016-10707 was assigned to track this issue.

anarcat commented Jan 18, 2018

CVE-2016-10707 was assigned to track this issue.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Jan 19, 2018

Member

@anarcat The CVE is incorrect. There is no stable jQuery version that suffers from this issue. The regression happened in preparation for jQuery 3.0.0 and it was fixed before 3.0.0 was released.

Member

mgol commented Jan 19, 2018

@anarcat The CVE is incorrect. There is no stable jQuery version that suffers from this issue. The regression happened in preparation for jQuery 3.0.0 and it was fixed before 3.0.0 was released.

@anarcat

This comment has been minimized.

Show comment
Hide comment
@anarcat

anarcat Jan 19, 2018

@mgol well, i based the CVE on the Snyk database which says "versions <3.0.0 >=2.1.0-beta1". i'm not sure I parsed those version numbers right, but i read that as releases after 2.1 (e.g. 2.2.2) being vulnerable.

i'd be happy to clarify the description in the CVE, once we have a better idea of what's going on here..

do you want to submit an update to the CVE or should i?

anarcat commented Jan 19, 2018

@mgol well, i based the CVE on the Snyk database which says "versions <3.0.0 >=2.1.0-beta1". i'm not sure I parsed those version numbers right, but i read that as releases after 2.1 (e.g. 2.2.2) being vulnerable.

i'd be happy to clarify the description in the CVE, once we have a better idea of what's going on here..

do you want to submit an update to the CVE or should i?

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Jan 19, 2018

Member

I've never submitted an update to a CVE before. :) Feel free to do it.

The range in the database is incorrect, the only affected version is 3.0.0-rc.1. Both the earlier 2.1.x, 2.2.x & 3.0.0-beta1 and the later 3.0.0 don't exhibit the issue.

Are CVEs assigned for bugs existing in pre-release versions? If so, the range should be changed to =3.0.0-rc.1.

Member

mgol commented Jan 19, 2018

I've never submitted an update to a CVE before. :) Feel free to do it.

The range in the database is incorrect, the only affected version is 3.0.0-rc.1. Both the earlier 2.1.x, 2.2.x & 3.0.0-beta1 and the later 3.0.0 don't exhibit the issue.

Are CVEs assigned for bugs existing in pre-release versions? If so, the range should be changed to =3.0.0-rc.1.

@anarcat

This comment has been minimized.

Show comment
Hide comment
@anarcat

anarcat Jan 19, 2018

acknowledged, i'll carry that over to mitre. i'm not sure this should have been marked as vulnerability in Snyke, and therefore in Mitre. i should have done my homework better but, arguably, there wasn't much information available to begin with, without digging deep in the source code. :)

For what it's worth, I requested a CVE through this form. I strongly encourage security researchers and upstream project to systematically request CVE assignments when discovering and/or releasing security issues. It makes tracking much easier across the ecosystem, from the upstream vendors down to all the downstream distributors and linux distros.

Thanks!

anarcat commented Jan 19, 2018

acknowledged, i'll carry that over to mitre. i'm not sure this should have been marked as vulnerability in Snyke, and therefore in Mitre. i should have done my homework better but, arguably, there wasn't much information available to begin with, without digging deep in the source code. :)

For what it's worth, I requested a CVE through this form. I strongly encourage security researchers and upstream project to systematically request CVE assignments when discovering and/or releasing security issues. It makes tracking much easier across the ecosystem, from the upstream vendors down to all the downstream distributors and linux distros.

Thanks!

ChALkeR added a commit to ChALkeR/retire.js that referenced this issue Mar 23, 2018

ChALkeR added a commit to ChALkeR/retire.js that referenced this issue Mar 23, 2018

ChALkeR added a commit to nodejs/security-wg that referenced this issue Mar 24, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Jul 18, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.