zf1-future: Error with sendmail: "Potential code injection in From header"

With the latest Version 1.22.0 of zf1-future I get the following error when using Sendmail:

Uncaught Zend_Mail_Transport_Exception: Potential code injection in From header in /var/www/project/vendor/shardj/zf1-future/library/Zend/Mail/Transport/Sendmail.php:125
Stack trace:
#0 /var/www/project/vendor/shardj/zf1-future/library/Zend/Mail/Transport/Abstract.php(348): Zend_Mail_Transport_Sendmail->_sendMail()
#1 /var/www/project/vendor/shardj/zf1-future/library/Zend/Mail.php(1201): Zend_Mail_Transport_Abstract->send()
#2 /var/www/project/library/Wlwp/Mail.php(580): Zend_Mail->send()
#3 /var/www/project/cronjobs/processScheduler.php(419): Wlwp_Mail->send()
#4 /var/www/project/cronjobs/processScheduler.php(66): TaskScheduler->_sendCronjobErrorMail()
#5 /var/www/project/cronjobs/processScheduler.php(8): TaskScheduler->process()
#6 {main}
  thrown in /var/www/project/vendor/shardj/zf1-future/library/Zend/Mail/Transport/Sendmail.php on line 125

The problem is the added validation of $this->parameters: https://github.com/Shardj/zf1-future/blob/6545737b8dbf8f4f3fa73b766b4dd3f6f0594730/library/Zend/Mail/Transport/Sendmail.php#L122-L135

According to the documentation of the mail()-function, the fifth parameter is for additional parameters to the sendmail program. So, while it is entirely possible that this string contains an email address, it is definitely not the only valid value, and I propose to remove this validation (and the replacement of spaces).

About this issue

Original URL
State: closed
Created a year ago
Comments: 22

Commits related to this issue

sendmail header sanitization quick-fix, as described in #326 — committed to develart-projects/zf1-future by develart-projects a year ago
Merge pull request #366 from develart-projects/sendmailEmailHeaderFix sendmail header sanitization quick-fix, as described in #326 — committed to Shardj/zf1-future by develart-projects a year ago
addressed 5th sendmail param validation using -f (#326) — committed to develart-projects/zf1-future by develart-projects 10 months ago
Merge pull request #371 from develart-projects/mailParametersValidationRework addressed 5th sendmail param validation using -f (#326) — committed to Shardj/zf1-future by develart-projects 10 months ago

Most upvoted comments

This will fix it: https://github.com/Shardj/zf1-future/pull/374

develart-projects on Aug 24, 2023

Option 2: If I got all correctly, we are looking for 3 double-quotes in the string, so this can be solution as well. As we are aiming to cover -f possibility, we can go for combined validation:

Do not validate, if empty
validate by Zend Mail validator, if option -f is not present
validate with more-than-2 double-quotes, if -f is used (or just for quotes, as ZF2, optionally)

By this, we are still covering most cases as they are now, but allowing to run it properly for the “-f” users as well.

Opening discussion before implementing anything. But not the long one, as I would like to close this one.

develart-projects on Aug 23, 2023

Option 1: ZF2 solution - double quotes validation

if (preg_match('/\\\"/', $address->getEmail())) {
            throw new Exception\RuntimeException("Potential code injection in From header");
        }

develart-projects on Aug 22, 2023

@yhabteab that’s actually what I was afraid about. Question is how to address that. I’ll release the quickfix, so will partially work. But will keep this one open for further development.

develart-projects on Aug 15, 2023