Page MenuHomePhabricator

Fifth argument to php mail() function should be better escaped.
Closed, ResolvedPublic

Description

I was reading https://blog.ripstech.com/2016/roundcube-command-execution-via-email/ . We don't do escaping of the fifth argument to mail() either. Our sanitation functions should prevent any evil mail addresses (Possibly someone could make something evil using the ! character, but its not clear if that is possible), but we should also just escape that similar to any other shell command.

Its somewhat complicated by the fact php does its own totally messed up escaping, so we can't just use wfEscapeShellArg.

For reference, line 409 of includes/mail/UserMailer.php

Event Timeline

Bawolff created this task.Dec 8 2016, 7:46 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 8 2016, 7:46 PM
Bawolff updated the task description. (Show Details)Dec 8 2016, 7:50 PM
Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".Dec 10 2016, 1:05 PM
Bawolff changed Security from Software security bug to None.
Bawolff added a project: Mail.

Making this public since its not exploitable and this is only a hardening measure.

Change 326260 had a related patch set uploaded (by Brian Wolff):
Escape return path extra params to php mail()

https://gerrit.wikimedia.org/r/326260

Bawolff triaged this task as Normal priority.Dec 13 2016, 9:53 PM

Change 326260 merged by jenkins-bot:
Escape return path extra params to php mail()

https://gerrit.wikimedia.org/r/326260

Did you want to backport this?

Yeah, we probably should. I can do the backports

Change 331663 had a related patch set uploaded (by Brian Wolff):
Escape return path extra params to php mail()

https://gerrit.wikimedia.org/r/331663

Change 331670 had a related patch set uploaded (by Brian Wolff):
Escape return path extra params to php mail()

https://gerrit.wikimedia.org/r/331670

Change 331663 merged by jenkins-bot:
Escape return path extra params to php mail()

https://gerrit.wikimedia.org/r/331663

Change 331670 merged by jenkins-bot:
Escape return path extra params to php mail()

https://gerrit.wikimedia.org/r/331670

Bawolff closed this task as Resolved.Feb 3 2017, 8:53 AM
Bawolff claimed this task.