tsolucio / corebos

core Business Operating System. An OPEN SOURCE business application that helps small and medium business handle all the day to day tasks.
https://corebos.com
150 stars 142 forks source link

Potential Catastrophic backtracking in regex #709

Open Luke1982 opened 4 years ago

Luke1982 commented 4 years ago

https://github.com/tsolucio/corebos/blob/5d3667847ae169f14f68a55160e501142a220997/modules/evvtgendoc/compile.php#L753

The test engine on https://regex101.com/ gave me this error when trying to use the regex (\w+)\s\[(.+)+\] on the sample strings:

{foreach CobroPago *date_start ASC*} {foreach CobroPago [paymentcategory=MyPaymentCat && credit=1]} {foreach CobroPago [paymentcategory=MyPaymentCat && credit=1] *date_start ASC*}

The article on https://www.regular-expressions.info/catastrophic.html then told me that groups that have quantifiers (like the + sign) that are itself again quantified (so (.+)+) are almost always catastrophic quantifiers.

Entering this data in regexpal will in fact, crash your browser. So probably, we want to improve this regex. Early tests point out that (\w+)\s\[(.+)\] (so without the group repetition quantifier) should also work.

Luke1982 commented 4 years ago

Note to self:

When altering the regex to allow sorting parameters to be passed to foreach, this could be it (\w+)\s(\*.+\*\s)?\[(.+)\]

That will allow: {foreach CobroPago [paymentcategory=MyPaymentCat && credit=1]} {foreach CobroPago [paymentcategory=MyPaymentCat && credit=1] *date_start DESC*} {foreach CobroPago *date_start ASC* [paymentcategory=MyPaymentCat && credit=1]}

to all be used and still pick up the conditions correctly.

joebordes commented 4 years ago

I agree with you. I'd say

(\w+)\s(\*.+\*\s)?\[(.+)\](\s\*.+\*)? if we want to support the sorting/condition anywhere (\w+)\s\[(.+)\](\s\*.+\*)? if we want only behind which makes "SQL-sense"