From 5736911a72ca5c09f2cf413a9c4b743d8e771d20 Mon Sep 17 00:00:00 2001 From: Laurence Date: Wed, 2 Aug 2017 21:48:32 +0000 Subject: [PATCH 1/2] improve protection --- .../Concerns/ValidatesAttributes.php | 4 +- tests/Validation/ValidationValidatorTest.php | 44 +++++++++---------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/Illuminate/Validation/Concerns/ValidatesAttributes.php b/src/Illuminate/Validation/Concerns/ValidatesAttributes.php index e80c78762bf1..88778dabf4f9 100644 --- a/src/Illuminate/Validation/Concerns/ValidatesAttributes.php +++ b/src/Illuminate/Validation/Concerns/ValidatesAttributes.php @@ -960,7 +960,9 @@ protected function shouldBlockPhpUpload($value, $parameters) return false; } - return strtolower($value->getExtension()) === 'php'; + return ($value instanceOf UploadedFile) + ? strtolower($value->getClientOriginalExtension()) === 'php' + : strtolower($value->getExtension()) === 'php'; } /** diff --git a/tests/Validation/ValidationValidatorTest.php b/tests/Validation/ValidationValidatorTest.php index c5c199b91f81..1180e9494e72 100755 --- a/tests/Validation/ValidationValidatorTest.php +++ b/tests/Validation/ValidationValidatorTest.php @@ -1950,39 +1950,39 @@ public function testValidateImage() $trans = $this->getIlluminateArrayTranslator(); $uploadedFile = [__FILE__, '', null, null, null, true]; - $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file->expects($this->any())->method('guessExtension')->will($this->returnValue('php')); - $file->expects($this->any())->method('getExtension')->will($this->returnValue('php')); + $file->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('php')); $v = new Validator($trans, ['x' => $file], ['x' => 'Image']); $this->assertFalse($v->passes()); - $file2 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file2 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file2->expects($this->any())->method('guessExtension')->will($this->returnValue('jpeg')); - $file2->expects($this->any())->method('getExtension')->will($this->returnValue('jpeg')); + $file2->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('jpeg')); $v = new Validator($trans, ['x' => $file2], ['x' => 'Image']); $this->assertTrue($v->passes()); - $file3 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file3 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file3->expects($this->any())->method('guessExtension')->will($this->returnValue('gif')); - $file3->expects($this->any())->method('getExtension')->will($this->returnValue('gif')); + $file3->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('gif')); $v = new Validator($trans, ['x' => $file3], ['x' => 'Image']); $this->assertTrue($v->passes()); - $file4 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file4 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file4->expects($this->any())->method('guessExtension')->will($this->returnValue('bmp')); - $file4->expects($this->any())->method('getExtension')->will($this->returnValue('bmp')); + $file4->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('bmp')); $v = new Validator($trans, ['x' => $file4], ['x' => 'Image']); $this->assertTrue($v->passes()); - $file5 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file5 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file5->expects($this->any())->method('guessExtension')->will($this->returnValue('png')); - $file5->expects($this->any())->method('getExtension')->will($this->returnValue('png')); + $file5->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('png')); $v = new Validator($trans, ['x' => $file5], ['x' => 'Image']); $this->assertTrue($v->passes()); - $file6 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file6 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file6->expects($this->any())->method('guessExtension')->will($this->returnValue('svg')); - $file6->expects($this->any())->method('getExtension')->will($this->returnValue('svg')); + $file6->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('svg')); $v = new Validator($trans, ['x' => $file6], ['x' => 'Image']); $this->assertTrue($v->passes()); } @@ -1992,9 +1992,9 @@ public function testValidateImageDoesNotAllowPhpExtensionsOnImageMime() $trans = $this->getIlluminateArrayTranslator(); $uploadedFile = [__FILE__, '', null, null, null, true]; - $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file->expects($this->any())->method('guessExtension')->will($this->returnValue('jpeg')); - $file->expects($this->any())->method('getExtension')->will($this->returnValue('php')); + $file->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('php')); $v = new Validator($trans, ['x' => $file], ['x' => 'Image']); $this->assertFalse($v->passes()); } @@ -2085,9 +2085,9 @@ public function testValidatePhpMimetypes() $trans = $this->getIlluminateArrayTranslator(); $uploadedFile = [__FILE__, '', null, null, null, true]; - $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file->expects($this->any())->method('guessExtension')->will($this->returnValue('rtf')); - $file->expects($this->any())->method('getExtension')->will($this->returnValue('rtf')); + $file->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('rtf')); $v = new Validator($trans, ['x' => $file], ['x' => 'mimetypes:text/*']); $this->assertTrue($v->passes()); @@ -2098,9 +2098,9 @@ public function testValidateMime() $trans = $this->getIlluminateArrayTranslator(); $uploadedFile = [__FILE__, '', null, null, null, true]; - $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file->expects($this->any())->method('guessExtension')->will($this->returnValue('pdf')); - $file->expects($this->any())->method('getExtension')->will($this->returnValue('pdf')); + $file->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('pdf')); $v = new Validator($trans, ['x' => $file], ['x' => 'mimes:pdf']); $this->assertTrue($v->passes()); @@ -2116,15 +2116,15 @@ public function testValidateMimeEnforcesPhpCheck() $trans = $this->getIlluminateArrayTranslator(); $uploadedFile = [__FILE__, '', null, null, null, true]; - $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file->expects($this->any())->method('guessExtension')->will($this->returnValue('pdf')); - $file->expects($this->any())->method('getExtension')->will($this->returnValue('php')); + $file->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('php')); $v = new Validator($trans, ['x' => $file], ['x' => 'mimes:pdf']); $this->assertFalse($v->passes()); - $file2 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getExtension'])->setConstructorArgs($uploadedFile)->getMock(); + $file2 = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')->setMethods(['guessExtension', 'getClientOriginalExtension'])->setConstructorArgs($uploadedFile)->getMock(); $file2->expects($this->any())->method('guessExtension')->will($this->returnValue('php')); - $file2->expects($this->any())->method('getExtension')->will($this->returnValue('php')); + $file2->expects($this->any())->method('getClientOriginalExtension')->will($this->returnValue('php')); $v = new Validator($trans, ['x' => $file2], ['x' => 'mimes:pdf,php']); $this->assertTrue($v->passes()); } From 73ddb7f8699eefee0f5871e3341e68acef76301f Mon Sep 17 00:00:00 2001 From: Laurence Date: Wed, 2 Aug 2017 21:53:40 +0000 Subject: [PATCH 2/2] style tweak --- src/Illuminate/Validation/Concerns/ValidatesAttributes.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Validation/Concerns/ValidatesAttributes.php b/src/Illuminate/Validation/Concerns/ValidatesAttributes.php index 88778dabf4f9..e2e6b893f092 100644 --- a/src/Illuminate/Validation/Concerns/ValidatesAttributes.php +++ b/src/Illuminate/Validation/Concerns/ValidatesAttributes.php @@ -960,7 +960,7 @@ protected function shouldBlockPhpUpload($value, $parameters) return false; } - return ($value instanceOf UploadedFile) + return ($value instanceof UploadedFile) ? strtolower($value->getClientOriginalExtension()) === 'php' : strtolower($value->getExtension()) === 'php'; }