bowser-js / bowser

a browser detector
Other
5.5k stars 487 forks source link

eslint-plugin-security marks bowser's regex expressions as unsafe #250

Open vorillaz opened 6 years ago

vorillaz commented 6 years ago

Hello there, this is a great project and thanks a lot for your work.

I am currently using bowser along with Express.js in order to create a logging system. In my setup I also use eslint-plugin-security as a primitive way to detect potential security issues. The eslint plugin though marks some of bowser's regex expressions as unsafe. I am currently sanitizing the headers passed through my app but still I am a bit worried. Any thoughts, tips or similar concerns will be highly appreciated.

lancedikson commented 6 years ago

Hi, @vorillaz. Thanks for the heads-up question. It seems like eslint-plugin-security uses safe-regex as the library that checks regexps on having abnormal star height. I'm not sure if it might be a security issue in any case or just the way to keep your regexps as simple as you can. But, I haven't got much experience in complexity measurement and regexp security analysis, so it's a great opportunity to dig dipper and learn new things.

I assume I'll read about that first of all and investigate if it's even an issue that users should be worried about and then we can come up with some plan on fixing the issues.

Have I understood you correctly, that now you're running eslint security check on the whole node_modules folder? If so, then, for now, I suggest you set the severity of those kinds of issues to warning level. If you or anybody else is highly interested in the investigation — welcome any time :)

vorillaz commented 6 years ago

Hey @lancedikson. Thanks for the quick response. I was a bit concerned as I am using Bower in order to parse User agents in order to process logs for a service for my app, thus I was a bit concerned about common security practices for Express.js apps. My way to go was limiting the length of the parsed User Agent to 256 characters. Bowser seems a great module and I was just asking if there are any other similar concerns from other users or maintainers. I would love to step up with a PR validating length of the User Agent and introducing the eslint-plugin-security as a way to go for development purposes. Afterwards, I would like to examine each regex in order to mark them as "secure", I am not quite experienced with regex static and security analysis so it would be a long shot for me too.

lancedikson commented 6 years ago

@vorillaz, everything you're saying sounds good to me, so please don't hesitate to investigate on the topic, discuss ones with the community and propose a PR 👍 I haven't found enough time to dig into the topic yet.

For the moment, I only have the following concerns: