mozilla / eslint-plugin-no-unsanitized

Custom ESLint rule to disallows unsafe innerHTML, outerHTML, insertAdjacentHTML and alike
Mozilla Public License 2.0
222 stars 33 forks source link

Unsupported Callee of type Literal for CallExpression #145

Closed denishowew closed 3 years ago

denishowew commented 3 years ago

Minimal code to reproduce: 0() This was derived from running addons-linter on code output by webpack, which seems to be minifying the bluebird library wrongly, which may be what is causing our browser extension to fail validation when we try to get it signed for Firefox.

mozfreddyb commented 3 years ago

I'm not sure how I can help you here. Why would you want to be able to lint code that won't run?

denishowew commented 3 years ago

It reproduces an error we get from code that does run despite this oddity, and addons-linter asked me to report it (sorry about the column layout):

UNSAFE_VAR_ASSIGNMENT   Error in                  Due to both security and performance           background.js             1      127817
                        no-unsanitized:           concerns, this may not be set using dynamic                                           
                        Unexpected Callee.        values which have not been adequately                                                 
                        Please report a minimal   sanitized. This can lead to security issues                                           
                        code snippet to the       or fairly serious performance degradation.                                            
                        developers at                                                                                                   
                        https://github.com/moz…                                                                                         
                        illa/eslint-plugin-no-…                                                                                         
                        unsanitized/issues/new…                                                                                         
                        ?title=Unsupported%20C…                                                                                         
                        allee%20of%20type%20Un…                                                                                         
                        aryExpression%20for%20…                                                                                         
                        CallExpression                                                                                                  

Shouldn't a linter tell you that your code is stupid rather than suggesting that the linter is broken?

mozfreddyb commented 3 years ago

A linter could tell you that your code is stupid. I don't know whether that linter should be eslint-plugin-no-unsanitized though.

mozfreddyb commented 3 years ago

First of all, I want to apologize for the tone I have been using above, that was not nice.

My point, however, was that I don't think it's legal JavaScript to call a number literal and I don't want us to have workarounds for incorrect JavaScript. That being said, I'd like to know more about this specific case and why it comes up - is it a bug in webpack? Is it legal JS and do we need to fix this?

Please let me know how you handled it so far and what you'd suggest we do here, @denishowew :)

denishowew commented 3 years ago

Your tone was lovely, it was me that was calling webpack's code stupid, if that's what you meant. But anyway, webpack is no longer producing this code and no sane person would so it's probably no big deal if it breaks the plugin, if indeed it still does. Let's close the ticket and pretend it never happened. :-)