fastify/fast-uri

LGTM-Alert Polynomial regular expression used on uncontrolled data

Uzlopak opened this issue · 8 comments

Uzlopak commented

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the issue has not already been raised

Issue

see:
https://lgtm.com/projects/g/fastify/fast-uri/snapshot/a91fe052eb5fc4f8831c7fee0131457289db14a9/files/index.js?sort=name&dir=ASC&mode=list#x852651248d66d755:1

mcollina commented

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

Uzlopak commented

I dont have a laptop the next 4 days, so if you want to wait that long, i could try. But i am not quite sure why you call match on not test.

I assume that you can fix it by doing, as the s-flag means that dot also matches newline
/\/\/(?:.)*:(?:\/|\?|#|$)/s

Or why do you need the non matching groups anyway,

Maybe the original writer of that regex can give more insights?

  • ❤️1
mcollina commented

Totally, take your time. cc @zekth

zekth commented

I'll try to investigate

zekth commented

This is relative to an old IE fix. I think we can drop the whole statement IMO

mcollina commented

Let's do it.

Fdawgs commented

This is relative to an old IE fix. I think we can drop the whole statement IMO

Got a link or some additional info on that @zekth? Just curious as to what it was fixing (I still have to support IE forever unfortunately).