Skip to content

Commit

Permalink
Fix for security vulnerability: Using the phar:// wrapper it was poss…
Browse files Browse the repository at this point in the history
…ible to trigger the unserialization of user provided data.
  • Loading branch information
nicolaasuni committed Sep 14, 2018
1 parent b32e75e commit 1861e33
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 32 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.TXT
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
6.2.20
- Fix for security vulnerability: Using the phar:// wrapper it was possible to trigger the unserialization of user provided data.

6.2.19
- Merge various fixes for PHP 7.3 compatibility and security.

Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "tecnickcom/tcpdf",
"version": "6.2.19",
"version": "6.2.20",
"homepage": "http://www.tcpdf.org/",
"type": "library",
"description": "TCPDF is a PHP class for generating PDF documents and barcodes.",
Expand Down
10 changes: 5 additions & 5 deletions include/tcpdf_fonts.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class TCPDF_FONTS {
* @public static
*/
public static function addTTFfont($fontfile, $fonttype='', $enc='', $flags=32, $outpath='', $platid=3, $encid=1, $addcbbox=false, $link=false) {
if (!file_exists($fontfile)) {
if (!TCPDF_STATIC::file_exists($fontfile)) {
// Could not find file
return false;
}
Expand All @@ -95,7 +95,7 @@ public static function addTTFfont($fontfile, $fonttype='', $enc='', $flags=32, $
$outpath = self::_getfontpath();
}
// check if this font already exist
if (@file_exists($outpath.$font_name.'.php')) {
if (@TCPDF_STATIC::file_exists($outpath.$font_name.'.php')) {
// this font already exist (delete it from fonts folder to rebuild it)
return $font_name;
}
Expand Down Expand Up @@ -1543,11 +1543,11 @@ public static function _getfontpath() {
public static function getFontFullPath($file, $fontdir=false) {
$fontfile = '';
// search files on various directories
if (($fontdir !== false) AND @file_exists($fontdir.$file)) {
if (($fontdir !== false) AND @TCPDF_STATIC::file_exists($fontdir.$file)) {
$fontfile = $fontdir.$file;
} elseif (@file_exists(self::_getfontpath().$file)) {
} elseif (@TCPDF_STATIC::file_exists(self::_getfontpath().$file)) {
$fontfile = self::_getfontpath().$file;
} elseif (@file_exists($file)) {
} elseif (@TCPDF_STATIC::file_exists($file)) {
$fontfile = $file;
}
return $fontfile;
Expand Down
4 changes: 2 additions & 2 deletions include/tcpdf_images.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ public static function _toJPEG($image, $quality, $tempfile) {
*/
public static function _parsejpeg($file) {
// check if is a local file
if (!@file_exists($file)) {
if (!@TCPDF_STATIC::file_exists($file)) {
// try to encode spaces on filename
$tfile = str_replace(' ', '%20', $file);
if (@file_exists($tfile)) {
if (@TCPDF_STATIC::file_exists($tfile)) {
$file = $tfile;
}
}
Expand Down
31 changes: 27 additions & 4 deletions include/tcpdf_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class TCPDF_STATIC {
* Current TCPDF version.
* @private static
*/
private static $tcpdf_version = '6.2.19';
private static $tcpdf_version = '6.2.20';

/**
* String alias for total number of pages.
Expand Down Expand Up @@ -1854,6 +1854,29 @@ public static function fopenLocal($filename, $mode) {
return fopen($filename, $mode);
}

/**
* Wrapper for file_exists.
* Checks whether a file or directory exists.
* Only allows some protocols and local files.
* @param filename (string) Path to the file or directory.
* @return Returns TRUE if the file or directory specified by filename exists; FALSE otherwise.
* @public static
*/
public static function file_exists($filename) {
if (strpos($filename, '://') > 0) {
$wrappers = stream_get_wrappers();
foreach ($wrappers as $wrapper) {
if (($wrapper === 'http') || ($wrapper === 'https')) {
continue;
}
if (stripos($filename, $wrapper.'://') === 0) {
return false;
}
}
}
return @file_exists($filename);
}

/**
* Reads entire file into a string.
* The file can be also an URL.
Expand Down Expand Up @@ -1914,8 +1937,10 @@ public static function fileGetContents($file) {
}
//
$alt = array_unique($alt);
//var_dump($alt);exit;//DEBUG
foreach ($alt as $path) {
if (!self::file_exists($path)) {
return false;
}
$ret = @file_get_contents($path);
if ($ret !== false) {
return $ret;
Expand Down Expand Up @@ -1949,8 +1974,6 @@ public static function fileGetContents($file) {
return false;
}



/**
* Get ULONG from string (Big Endian 32-bit unsigned integer).
* @param $str (string) string from where to extract value
Expand Down
31 changes: 11 additions & 20 deletions tcpdf.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php
//============================================================+
// File name : tcpdf.php
// Version : 6.2.19
// Version : 6.2.20
// Begin : 2002-08-03
// Last Update : 2018-09-14
// Author : Nicola Asuni - Tecnick.com LTD - www.tecnick.com - info@tecnick.com
Expand Down Expand Up @@ -104,7 +104,7 @@
* Tools to encode your unicode fonts are on fonts/utils directory.</p>
* @package com.tecnick.tcpdf
* @author Nicola Asuni
* @version 6.2.19
* @version 6.2.20
*/

// TCPDF configuration
Expand All @@ -128,7 +128,7 @@
* TCPDF project (http://www.tcpdf.org) has been originally derived in 2002 from the Public Domain FPDF class by Olivier Plathey (http://www.fpdf.org), but now is almost entirely rewritten.<br>
* @package com.tecnick.tcpdf
* @brief PHP class for generating PDF documents without requiring external extensions.
* @version 6.2.19
* @version 6.2.20
* @author Nicola Asuni - info@tecnick.com
* @IgnoreAnnotation("protected")
* @IgnoreAnnotation("public")
Expand Down Expand Up @@ -4256,7 +4256,7 @@ public function AddFont($family, $style='', $fontfile='', $subset='default') {
// true when the font style variation is missing
$missing_style = false;
// search and include font file
if (TCPDF_STATIC::empty_string($fontfile) OR (!@file_exists($fontfile))) {
if (TCPDF_STATIC::empty_string($fontfile) OR (!@TCPDF_STATIC::file_exists($fontfile))) {
// build a standard filenames for specified font
$tmp_fontfile = str_replace(' ', '', $family).strtolower($style).'.php';
$fontfile = TCPDF_FONTS::getFontFullPath($tmp_fontfile, $fontdir);
Expand All @@ -4268,7 +4268,7 @@ public function AddFont($family, $style='', $fontfile='', $subset='default') {
}
}
// include font file
if (!TCPDF_STATIC::empty_string($fontfile) AND (@file_exists($fontfile))) {
if (!TCPDF_STATIC::empty_string($fontfile) AND (@TCPDF_STATIC::file_exists($fontfile))) {
include($fontfile);
} else {
$this->Error('Could not include font definition file: '.$family.'');
Expand Down Expand Up @@ -4809,19 +4809,19 @@ public function Annotation($x, $y, $w, $h, $text, $opt=array('Subtype'=>'Text'),
$this->PageAnnots[$page][] = array('n' => ++$this->n, 'x' => $x, 'y' => $y, 'w' => $w, 'h' => $h, 'txt' => $text, 'opt' => $opt, 'numspaces' => $spaces);
if (!$this->pdfa_mode) {
if ((($opt['Subtype'] == 'FileAttachment') OR ($opt['Subtype'] == 'Sound')) AND (!TCPDF_STATIC::empty_string($opt['FS']))
AND (@file_exists($opt['FS']) OR TCPDF_STATIC::isValidURL($opt['FS']))
AND (@TCPDF_STATIC::file_exists($opt['FS']) OR TCPDF_STATIC::isValidURL($opt['FS']))
AND (!isset($this->embeddedfiles[basename($opt['FS'])]))) {
$this->embeddedfiles[basename($opt['FS'])] = array('f' => ++$this->n, 'n' => ++$this->n, 'file' => $opt['FS']);
}
}
// Add widgets annotation's icons
if (isset($opt['mk']['i']) AND @file_exists($opt['mk']['i'])) {
if (isset($opt['mk']['i']) AND @TCPDF_STATIC::file_exists($opt['mk']['i'])) {
$this->Image($opt['mk']['i'], '', '', 10, 10, '', '', '', false, 300, '', false, false, 0, false, true);
}
if (isset($opt['mk']['ri']) AND @file_exists($opt['mk']['ri'])) {
if (isset($opt['mk']['ri']) AND @TCPDF_STATIC::file_exists($opt['mk']['ri'])) {
$this->Image($opt['mk']['ri'], '', '', 0, 0, '', '', '', false, 300, '', false, false, 0, false, true);
}
if (isset($opt['mk']['ix']) AND @file_exists($opt['mk']['ix'])) {
if (isset($opt['mk']['ix']) AND @TCPDF_STATIC::file_exists($opt['mk']['ix'])) {
$this->Image($opt['mk']['ix'], '', '', 0, 0, '', '', '', false, 300, '', false, false, 0, false, true);
}
}
Expand Down Expand Up @@ -6845,20 +6845,11 @@ public function Image($file, $x='', $y='', $w=0, $h=0, $type='', $link='', $alig
$file = substr($file, 1);
$exurl = $file;
}
$wrappers = stream_get_wrappers();
foreach ($wrappers as $wrapper) {
if ($wrapper === 'http' || $wrapper === 'https') {
continue;
}
if (stripos($file, $wrapper.'://') === 0) {
$this->Error('Stream wrappers in file paths are not supported');
}
}
// check if is a local file
if (!@file_exists($file)) {
if (!@TCPDF_STATIC::file_exists($file)) {
// try to encode spaces on filename
$tfile = str_replace(' ', '%20', $file);
if (@file_exists($tfile)) {
if (@TCPDF_STATIC::file_exists($tfile)) {
$file = $tfile;
}
}
Expand Down

8 comments on commit 1861e33

@leofeyer
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid this change is wrong. file_exists() is not the only method vulnerable for this kind of attack. It is also the getimagesize() call in line 6856, which can now be exploited again due to your deletions of lines 6848 to 6856 (my fix from #94).

@nicolaasuni
Copy link
Member Author

Choose a reason for hiding this comment

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

I think I have fixed this in 6.2.22.
Thanks for reporting this.

@leofeyer
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you @nicolaasuni. Version 6.2.22 prevents the attack again. 👍

@abergmann
Copy link

Choose a reason for hiding this comment

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

CVE-2018-17057 was assigned to this issue.

@jeroenvermeulen
Copy link

Choose a reason for hiding this comment

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

@yybalam
Copy link

Choose a reason for hiding this comment

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

This change broke my phar. O.O
If I pack my project into a phar then TCPDF respond "Could not include font definition file: helvetica.", otherwise work fine. So, what is the right way for pack into a phar the TCPDF now?

@jasonzom
Copy link

Choose a reason for hiding this comment

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

Hello i use 6.2.0 version and used scanner and it shows affected as well is there a exploit it ?

@NiklasBr
Copy link
Contributor

Choose a reason for hiding this comment

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

@jasonzom any day there is a security issue revealed you should assume there was a high likelihood that some bad actor already discovered it before. And after the issue and fix had been revealed then it is pretty much a certainty that there are exploits available. You should really update.

Please sign in to comment.