[3.10] gh-68966: Make mailcap refuse to match unsafe filenames/types/params (GH-91993)

[miss-islington]

(cherry picked from commit b9509ba)

Co-authored-by: Petr Viktorin encukou@gmail.com

• Issue: [CVE-2015-20107] mailcap.findmatch: document shell command Injection danger in filename parameter #68966

[encukou]

@gpshead said:

The failure scenario for an application where we simply start claiming no match (return None?) on potentially suspicious filenames seems acceptable. I doubt it would be a big deal to users if any even notice at all. So if you want to go forward with a PR like #91993 my gut feeling is that nobody is going to balk at the change, even in security only release branches.

@pablogsal, do you agree?

[pablogsal]

@gpshead said:

The failure scenario for an application where we simply start claiming no match (return None?) on potentially suspicious filenames seems acceptable. I doubt it would be a big deal to users if any even notice at all. So if you want to go forward with a PR like #91993 my gut feeling is that nobody is going to balk at the change, even in security only release branches.

@pablogsal, do you agree?

I agree, but I am still not confident on backporting it, so unless there is some clear consensus from everyone I would recommend to be cautious here.

[miss-islington]

Status check is done, and it's a success ✅ .

[encukou]

Who's "everyone"?
Not too many people are interested in mailcap :)

[pablogsal]

Everyone is any core Dev interested on mailcap that want to voice their opinion. If nobody objects or everyone is just you and @gpshead then go ahead and merge it :)

[encukou]

So let's ping core devs from the original issue – but I doubt even they are particularly interested in mailcap.

@zooba @brettcannon @vstinner, do you agree with Greg?

The failure scenario for an application where we simply start claiming no match (return None?) on potentially suspicious filenames seems acceptable. I doubt it would be a big deal to users if any even notice at all. So if you want to go forward with a PR like [this] my gut feeling is that nobody is going to balk at the change, even in security only release branches.

[brettcannon]

So let's ping core devs from the original issue – but I doubt even they are particularly interested in mailcap.

Correct, I am not interested and thus have no opinion. 😁

[zooba]

My only strong preference is that there should be a clear what's new entry that specifies at least the name of the warning, to give users encountering this the best possible chance to figure out what has changed (and that it was us, and not something that they did).

[prashant1221]

Will this PR be merged in 3.10? Can we resolve the issues.

[encukou]

Sorry, I missed the notifications here :(

Do we add What's new entries for point releases? I don't think I've seen one, but then I usually only read What's New for the .0 version.

@prashant1221, what issues are you having? Do you actually use mailcap?

[prashant1221]

Sorry, I missed the notifications here :(

Do we add What's new entries for point releases? I don't think I've seen one, but then I usually only read What's New for the .0 version.

@prashant1221, what issues are you having? Do you actually use mailcap?

No, it was to close the CVE-2015-20107 in our distro, Fedora seems to have taken this patch on all python3 branches.

[vstinner]

Do we add What's new entries for point releases? I don't think I've seen one, but then I usually only read What's New for the .0 version.

Yes, we do, especially for security fixes. Recent example: https://docs.python.org/dev/whatsnew/3.9.html#notable-changes-in-python-3-9-2

[vstinner]

@encukou: Oh, this PR is not merged yet! Python 3.10 is still vulnerable, as older Python versions (3.7, 3.8 and 3.9 which still accept security fixes).

[gpshead]

I added what's new text and bumped the versionchanged stuff to 3.10.8.

[encukou]

Sorry for dropping the ball – this was #29 on my TODO list, which ain't a good spot :(
Thanks for the ping.