Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

MimeType validator possibly leads to memory exhausted fatal error #207

Open
sserbin opened this issue Nov 30, 2017 · 1 comment
Open

MimeType validator possibly leads to memory exhausted fatal error #207

sserbin opened this issue Nov 30, 2017 · 1 comment

Comments

@sserbin
Copy link

sserbin commented Nov 30, 2017

Test case:

<?php

include 'vendor/autoload.php';

ini_set('memory_limit', '16M'); // might also happen with default 128M, lowering it here just to ensure test fail

$mimeValidator = new Zend\Validator\File\MimeType(['text/plain']);

file_put_contents('/tmp/foobar', 'test');

var_dump($mimeValidator->isValid('/tmp/foobar'));

unlink('/tmp/foobar');

The problem is finfo_open'ing magic file can be quite noisy (producing a whole lot of warnings) when it isn't super satisfied with a given magic file - albeit still working.
This and the fact the mime validator utilizes Zend\Stdlib\ErrorHandler to suppress notices/warnings - which, under the hood, keeps track of all the exceptions occurred (until the stop is invoked) - could lead to a memory exhausted fatal in memory-limited environment.

Tested on Ubuntu 16.04/17.04 php-7.1.1

I think one of the possible "fixes" (reliability improvement) might be to set an actual explicit ignore error handler (and not use the generic ErrorHandler::start ignoring it's results afterwards)
So, instead of doing:

                ErrorHandler::start(E_NOTICE | E_WARNING);
                $this->finfo = finfo_open(FILEINFO_MIME_TYPE, $mimefile);
                ErrorHandler::stop();

the errorHandler could be explicitly requested to ignore the errors of given levels: ErrorHandler::ignore(E_NOTICE | E_WARNING);

This would however require changing of Stdlib/ErroHandler as it doesn't provide the ignore functionality atm.

Not only this would prevent the aforementioned use-case failure, it would both have improvements on performance (saving some memory and cpu cycles) and express the intention more clearly in the code.

@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-validator; a new issue has been opened at laminas/laminas-validator#10.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants