Skip to content
Permalink
Browse files Browse the repository at this point in the history
Proposed fix for #2069
  • Loading branch information
Synchro committed Sep 30, 2020
1 parent 59dea8c commit e2e07a3
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
25 changes: 21 additions & 4 deletions src/PHPMailer.php
Expand Up @@ -1753,6 +1753,23 @@ protected static function isPermittedPath($path)
return !preg_match('#^[a-z]+://#i', $path);
}

/**
* Check whether a file path is safe, accessible, and readable.
*
* @param string $path A relative or absolute path to a file
*
* @return bool
*/
protected static function fileIsAccessible($path)
{
$readable = file_exists($path);
//If not a UNC path (expected to start with \\), check read permission, see #2069
if (strpos($path, '\\\\') !== 0) {
$readable = $readable && is_readable($path);
}
return static::isPermittedPath($path) && $readable;
}

/**
* Send mail using the PHP mail() function.
*
Expand Down Expand Up @@ -2141,7 +2158,7 @@ public function setLanguage($langcode = 'en', $lang_path = '')
// There is no English translation file
if ('en' !== $langcode) {
// Make sure language file path is readable
if (!static::isPermittedPath($lang_file) || !file_exists($lang_file)) {
if (!static::fileIsAccessible($lang_file)) {
$foundlang = false;
} else {
// Overwrite language-specific strings.
Expand Down Expand Up @@ -2970,7 +2987,7 @@ public function addAttachment(
$disposition = 'attachment'
) {
try {
if (!static::isPermittedPath($path) || !@is_file($path) || !is_readable($path)) {
if (!static::fileIsAccessible($path)) {
throw new Exception($this->lang('file_access') . $path, self::STOP_CONTINUE);
}

Expand Down Expand Up @@ -3144,7 +3161,7 @@ protected function attachAll($disposition_type, $boundary)
protected function encodeFile($path, $encoding = self::ENCODING_BASE64)
{
try {
if (!static::isPermittedPath($path) || !file_exists($path) || !is_readable($path)) {
if (!static::fileIsAccessible($path)) {
throw new Exception($this->lang('file_open') . $path, self::STOP_CONTINUE);
}
$file_buffer = file_get_contents($path);
Expand Down Expand Up @@ -3530,7 +3547,7 @@ public function addEmbeddedImage(
$disposition = 'inline'
) {
try {
if (!static::isPermittedPath($path) || !@is_file($path) || !is_readable($path)) {
if (!static::fileIsAccessible($path)) {
throw new Exception($this->lang('file_access') . $path, self::STOP_CONTINUE);
}

Expand Down
3 changes: 3 additions & 0 deletions test/PHPMailerTest.php
Expand Up @@ -1476,6 +1476,9 @@ public function testAltBodyAttachment()
return;
}

//Test using non-existent UNC path
self::assertFalse($this->Mail->addAttachment('\\\\nowhere\nothing'));

$this->buildBody();
self::assertTrue($this->Mail->send(), $this->Mail->ErrorInfo);
}
Expand Down

1 comment on commit e2e07a3

@broberts-dev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marcus: I tested this commit () in one of non-prod. instances and the issue attaching files specific by a UNC path appears to be resolved.

Please sign in to comment.