Snyk path traversal vulnerability

[mitchhankins01]

When running vulnerability testing using Snyk on the TCPDF library, we are alerted of a high path traversal vulnerability in "tcpdf_static.php" on line 1949

$ret = @file_get_contents (Spath);

according to Snyk: "Unsanitized input from an HTTP header flows into file_get_contents, where it is used as a path. This may result in a Path Traversal vulnerability and allow an attacker to read arbitrary files."

Can you advise on whether this is truly a vulnerability? We are unable to modify the library due to the LGPL license.

Thank you!

[williamdes]

We are unable to modify the library due to the LGPL license.

I do not understand why this is an issue, can you elaborate ?

The function seems not used by the code as far as my research confirms it.
See: https://github.com/search?q=repo%3Atecnickcom%2FTCPDF%20fileGetContents&type=code
Ref:

TCPDF/include/tcpdf_static.php

Line 1894 in d4adef4

public static function fileGetContents($file) {

@nicolaasuni what about deleting it and some more stuff like tcpdf import and releasing a major version ?

[mitchhankins01]

Hi @williamdes, thank you for your response.
We are unable to modify the library (or eliminate the unused function) because it is used in our proprietary software.
A new release with the function taken out would be greatly appreciated!

[kmurph92]

If removal of the offending function is not possible or undesirable, I recommend considering the use of the realpath and basename functions to prevent possible path traversal.

If you believe that path traversal is not possible within this function, can you please let us know why it is not possible?

Thanks!

[williamdes]

Well, I will let @nicolaasuni take the decisions needed or not. I am only a contributor to this project.
For me this is a theoretical security issue. If you can provide proof of a real one I may work on a fix

[kmurph92]

Thanks for your reply williamdes. We believe the following call (and ones like it) may be used to expose sensitive information from the file system:

TCPDF_STATIC::fileGetContents('../../../etc/passwd');

If you disagree with our belief, please let us know why the behavior I described isn't possible.

For reference, you can find information about path traversal vulnerabilities, including attack examples and mitigation strategies at OWASP website here: https://owasp.org/www-community/attacks/Path_Traversal

Thanks Again.

[d-javu]

The LGPL argument is a strange one. Do the work, send your changes upstream. Just as for any other open source software.

The mentioned function is used by the file content cache:

TCPDF/tcpdf.php

Lines 24765 to 24771 in d4adef4

	protected function getCachedFileContents($file)
	{
	if (!isset($this->fileContentCache[$file])) {
	$this->fileContentCache[$file] = TCPDF_STATIC::fileGetContents($file);
	}
	return $this->fileContentCache[$file];
	}

It is called from:

_putEmbeddedFiles()
getHtmlDomArray($html)
Image($file, ...)
ImageEps($file, ...)
ImageSVG($file, ...)

I've not investigated thoroughly, but it looks difficult to get data out of /random/secret/file using these functions.

[kmurph92]

Thanks d-javu. We will plan to submit a pull request soon.

[mitchhankins01]

We have discovered that the "HOST" header of an HTTP request is immutable by the end user. The host will be whatever the domain name is that the HTTP request is being sent to. The TCPDF_STATIC::fileGetContents function is not subject to any mutable HTTP header information and therefore, this can be classified as a false positive.