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

Bind polyfill still needed? It has been flagged as 'exposed dangerous method or function' #2751

Closed
paulmsmith opened this issue Aug 8, 2022 · 1 comment
Labels
security validation errors Help users to recover from validation errors

Comments

@paulmsmith
Copy link

paulmsmith commented Aug 8, 2022

Developers in my service team have informed me the 'bind' polyfill has been flagged in some automated security/threat testing as Common Weakness Enumeration 749 because of line 125.

bound = Function('binder', 'return function (' + boundArgs.join(',') + '){ return binder.apply(this, arguments); }')(binder);

We think it is upset because that bit of code is effectively an inline 'eval' which technically could be used as part of a cross-site scripting exploit.

I thought it might be worth adding this issue to help others.

I wonder if the polyfill is still needed actually? CanIUse suggests to me that this polyfill is for IE6 - IE8 and I think the GOVUK Design System team are now working to drop support for IE8?

@querkmachine querkmachine added the awaiting triage Needs triaging by team label Aug 8, 2022
@querkmachine querkmachine added validation errors Help users to recover from validation errors security 🕔 days and removed awaiting triage Needs triaging by team labels Aug 10, 2022
@querkmachine
Copy link
Member

Hi @paulmsmith,

You're correct that this is a polyfill for IE8, which we still support in GOV.UK Frontend. We're still at least a couple of months out from dropping support for it completely, but it will eventually be removed as part of #2506.

The polyfill we use is derived from Polyfill.io. Their version of the code is here, though they have since removed support for IE8 and this polyfill with it, so it's unlikely that they will respond to a security report.

Although it being flagged as a security issue isn't great, this code will only be executed by IE8 and similarly old browsers outside of our support remit, which feels like it mitigates the risk of a serious or widespread security threat.

I've added this to our list of known validation warnings for now, but I don't think it's likely we'll fix this before we remove polyfills anyway.

@querkmachine querkmachine closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security validation errors Help users to recover from validation errors
Development

No branches or pull requests

2 participants