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

Assigned To

Authored By

	Bawolff
	Dec 8 2016, 7:46 PM

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

Details

Subject	Repo	Branch	Lines +/-
Escape return path extra params to php mail()	mediawiki/core	REL1_27	+9 -1
Escape return path extra params to php mail()	mediawiki/core	REL1_28	+9 -1
Escape return path extra params to php mail()	mediawiki/core	master	+8 -1

Customize query in gerrit

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

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

Ciencia_Al_Poder updated the task description. (Show Details)Dec 10 2016, 1:08 PM

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

gerritbot added a project: Patch-For-Review.Dec 10 2016, 1:20 PM

Bawolff triaged this task as Medium 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?

ReleaseTaggerBot added projects: MW-1.29-release (WMF-deploy-2017-01-03_(1.29.0-wmf.7)), MW-1.29-release-notes.Dec 15 2016, 7:00 AM

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

ReleaseTaggerBot added projects: MW-1.28-release-notes, MW-1.27-release-notes.Jan 11 2017, 8:00 PM

SamanthaNguyen moved this task from Backlog / Other to Patch pending review on the acl*security board.Jan 23 2017, 4:39 AM

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

Bawolff claimed this task.

Wikinaut subscribed.May 3 2017, 11:02 PM

sbassett moved this task from Patch pending review to Done on the acl*security board.Sep 26 2019, 2:45 PM

• chasemp added a project: Security.Feb 10 2020, 10:58 PM

• chasemp removed a project: acl*security.Feb 20 2020, 8:17 PM

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

Description

Details

Event Timeline

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