As designed Uploaded image dimensions are never checked

[TickTackk]

Affected version

2.1

If the uploaded image dimensions is larger than allowed, it gets resized in transformImage() which is called before checking image dimensions.

So in file: \src\XF\Http\Upload.php method isValid() should look something like this

    public function isValid(&$errors = [])
    {
        $isImage = $this->isImage();
        $errors = [];

        if ($isImage)
        {
            if ($this->imageContentUnsafe)
            {
                $errors['content'] = \XF::phrase('uploaded_image_contains_invalid_content');
            }

            if (
                ($this->maxWidth && $this->imageWidth > $this->maxWidth)
                || ($this->maxHeight && $this->imageHeight > $this->maxHeight)
            )
            {
                $errors['dimensions'] = \XF::phrase('uploaded_image_is_too_big');
            }
        }

        $this->transformImage();

        if ($this->uploadError)
        {
            $errors['server'] = $this->getServerUploadError();
            return false;
        }

        if (!$this->tempFile)
        {
            $errors['server'] = \XF::phrase('uploaded_file_failed_not_found');
            return false;
        }

        if (!$this->fileSize)
        {
            $errors['fileSize'] = \XF::phrase('uploaded_file_empty_please_try_a_different_file');
            return false;
        }

        $isVideo = $this->isVideo();

        if ($this->imageRequired && !$isImage)
        {
            $errors['image'] = \XF::phrase('uploaded_file_must_be_valid_image');
        }

        if ($this->allowedExtensions && !in_array($this->extension, $this->allowedExtensions))
        {
            $errors['extension'] = \XF::phrase('uploaded_file_does_not_have_an_allowed_extension');
        }
        else if (!$this->isImage && $this->hasImageExtension())
        {
            $errors['extension'] = \XF::phrase('the_uploaded_file_was_not_an_image_as_expected');
        }
        else if (!$this->isVideo && $this->hasVideoExtension() && $this->requireValidVideo)
        {
            $errors['extension'] = \XF::phrase('the_uploaded_file_was_not_a_video_as_expected');
        }

        if ($isVideo)
        {
            if ($this->maxVideoSize && $this->fileSize > $this->maxVideoSize)
            {
                $errors['fileSize'] = \XF::phrase('uploaded_file_is_too_large');
            }
        }
        else
        {
            if ($this->maxFileSize && $this->fileSize > $this->maxFileSize)
            {
                $errors['fileSize'] = \XF::phrase('uploaded_file_is_too_large');
            }
        }

        $errors = array_merge($this->extraErrors, $errors);

        return count($errors) == 0;
    }

 

[Chris D]

Can you please be more clear about the issues this is actually causing?

Because as far as I can tell, you're suggesting that we always reject images that exceed the maximum width/height - that's not at all what the aim of the max width/height is. The whole idea of the max width/height is we attempt to resize it below those maximums before checking it.

 

[TickTackk]

I always thought the option was to reject any image that was larger than required

 

[Chris D]

Nope.