REDoS in purge

[yetingli]

BPO	41712
Nosy	@pfmoore, @tjguk, @zware, @serhiy-storchaka, @zooba, @yetingli, @Pixmew
PRs	bpo-41712: Vulnerable regex changed #23166 bpo-41712 Vulnerable Regex Changed #23191 bpo-41712 Vulnerable Regex #23287
Files	purge.py purge.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-11-09.19:14:23.592>
created_at = <Date 2020-09-04.09:47:22.735>
labels = ['easy', 'type-bug', 'expert-installation', '3.10', 'OS-windows']
title = 'REDoS in purge'
updated_at = <Date 2020-11-14.17:38:53.875>
user = 'https://github.com/yetingli'

bugs.python.org fields:

activity = <Date 2020-11-14.17:38:53.875>
actor = 'Pixmew'
assignee = 'none'
closed = True
closed_date = <Date 2020-11-09.19:14:23.592>
closer = 'steve.dower'
components = ['Installation', 'Windows']
creation = <Date 2020-09-04.09:47:22.735>
creator = 'yetingli'
dependencies = []
files = ['49443', '49572']
hgrepos = []
issue_num = 41712
keywords = ['patch', 'easy']
message_count = 8.0
messages = ['376343', '376356', '376377', '376385', '380411', '380413', '380601', '380610']
nosy_count = 7.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'serhiy.storchaka', 'steve.dower', 'yetingli', 'Pixmew']
pr_nums = ['23166', '23191', '23287']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue41712'
versions = ['Python 3.10']

[yetingli]

I find this regex "(\d+\.\d+\.\d+)(\w+\d+)?$" may be stucked by input.
The vulnerable regex is located in

cpython/Tools/msi/purge.py

Line 15 in 54a66ad

VERSION_RE = re.compile(r'(\d+\.\d+\.\d+)(\w+\d+)?$')

The ReDOS vulnerability of the regex is mainly due to the sub-pattern \w+\d+
and can be exploited with the following string
"1.1.1"+"1" * 5000 + "!"

I think you can limit the input length or fix this regex.

For example, you can modify the sub-pattern \w+\d+ to ([A-Za-z_]*\d)+

Looking forward for your response​!

Best,
Yeting Li

[serhiy-storchaka]

Thank you for your report yeting li. The pattern modification looks good to me. Do you mind to create a pull request?

[zware]

Does it matter? This is not a library, it is a script used occasionally by a release manager, called manually, and the only input to the regex is provided via a command-line argument in that manual call. I don't think Steve plans to REDoS himself :)

[zooba]

I've considered DoSing myself a few times, but then change my mind and just publish the release :)

A PR to change it to "(\d+\.\d+\.\d+)([a-zA-Z]+\d+)?$" would be fine, but is not urgent. It certainly doesn't need to be backported, as this is only ever used from master these days.

Personally I'd be just as happy closing the issue. I know that the current script works, and there's nothing worse than breaking a release because someone has changed the release scripts without testing them properly.