From fbd050d4e447328881e0925371bd266dcd55c907 Mon Sep 17 00:00:00 2001 From: Attila Kerekes <439392+keriati@users.noreply.github.com> Date: Tue, 6 Jun 2023 12:08:47 +0200 Subject: [PATCH] fix: validate icons to be images (#1173) --- app/Helper.php | 27 ++++++++------ app/Http/Controllers/ItemController.php | 28 +++++++-------- tests/Unit/helpers/IsImageTest.php | 34 +++++++++--------- .../fixtures/heimdall-icon-small-php.php | Bin 5362 -> 0 bytes 4 files changed, 47 insertions(+), 42 deletions(-) delete mode 100644 tests/Unit/helpers/fixtures/heimdall-icon-small-php.php diff --git a/app/Helper.php b/app/Helper.php index f7fcd5e3..abb9cfdb 100644 --- a/app/Helper.php +++ b/app/Helper.php @@ -14,15 +14,15 @@ function format_bytes($bytes, bool $is_drive_size = true, string $beforeunit = ' $btype = ($is_drive_size === true) ? 1000 : 1024; $labels = ['B', 'KB', 'MB', 'GB', 'TB']; // use 1000 rather than 1024 to simulate HD size not real size - for ($x = 0; $bytes >= $btype && $x < (count($labels) - 1); $bytes /= $btype, $x++); + for ($x = 0; $bytes >= $btype && $x < (count($labels) - 1); $bytes /= $btype, $x++) ; if ($labels[$x] == 'TB') { - return round($bytes, 3).$beforeunit.$labels[$x].$afterunit; + return round($bytes, 3) . $beforeunit . $labels[$x] . $afterunit; } elseif ($labels[$x] == 'GB') { - return round($bytes, 2).$beforeunit.$labels[$x].$afterunit; + return round($bytes, 2) . $beforeunit . $labels[$x] . $afterunit; } elseif ($labels[$x] == 'MB') { - return round($bytes, 2).$beforeunit.$labels[$x].$afterunit; + return round($bytes, 2) . $beforeunit . $labels[$x] . $afterunit; } else { - return round($bytes, 0).$beforeunit.$labels[$x].$afterunit; + return round($bytes, 0) . $beforeunit . $labels[$x] . $afterunit; } } @@ -37,11 +37,11 @@ function str_slug($title, string $separator = '-', string $language = 'en'): str return Str::slug($title, $separator, $language); } -if (! function_exists('str_is')) { +if (!function_exists('str_is')) { /** * Determine if a given string matches a given pattern. * - * @param string|array $pattern + * @param string|array $pattern * @param string $value * @return bool * @@ -64,7 +64,7 @@ function get_brightness($hex) // $hex = str_replace('#', '', $hex); $hex = preg_replace("/[^0-9A-Fa-f]/", '', $hex); if (strlen($hex) == 3) { - $hex = $hex[0].$hex[0].$hex[1].$hex[1].$hex[2].$hex[2]; + $hex = $hex[0] . $hex[0] . $hex[1] . $hex[1] . $hex[2] . $hex[2]; } $c_r = hexdec(substr($hex, 0, 2)); @@ -97,7 +97,7 @@ function getLinkTargetAttribute(): string if ($target === 'current') { return ''; } else { - return ' target="'.$target.'"'; + return ' target="' . $target . '"'; } } @@ -112,10 +112,17 @@ function className($name) /** * @param string $file + * @param string $extension * @return bool */ -function isImage(string $file):bool +function isImage(string $file, string $extension): bool { + $allowedExtensions = ['jpg', 'jpeg', 'png', 'bmp', 'gif', 'svg', 'webp']; + + if (!in_array($extension, $allowedExtensions)) { + return false; + } + $tempFileName = tempnam("/tmp", "image-check-"); $handle = fopen($tempFileName, "w"); diff --git a/app/Http/Controllers/ItemController.php b/app/Http/Controllers/ItemController.php index 20941e7f..86e1c491 100644 --- a/app/Http/Controllers/ItemController.php +++ b/app/Http/Controllers/ItemController.php @@ -141,7 +141,7 @@ class ItemController extends Controller */ public function index(Request $request): View { - $trash = (bool) $request->input('trash'); + $trash = (bool)$request->input('trash'); $data['apps'] = Item::ofType('item')->orderBy('title', 'asc')->get(); $data['trash'] = Item::ofType('item')->onlyTrashed()->get(); @@ -197,6 +197,7 @@ class ItemController extends Controller * @param Request $request * @param null $id * @return Item + * @throws ValidationException */ public static function storelogic(Request $request, $id = null): Item { @@ -219,21 +220,18 @@ class ItemController extends Controller "verify_peer_name" => false, ), ); + + $file = $request->input('icon'); + $path_parts = pathinfo($file); + $extension = $path_parts['extension']; + $contents = file_get_contents($request->input('icon'), false, stream_context_create($options)); - if (!isImage($contents)) { + if (!isImage($contents, $extension)) { throw ValidationException::withMessages(['file' => 'Icon must be an image.']); } - if ($application) { - $icon = $application->icon; - } else { - $file = $request->input('icon'); - $path_parts = pathinfo($file); - $icon = md5($contents); - $icon .= '.' . $path_parts['extension']; - } - $path = 'icons/' . $icon; + $path = 'icons/' . ($application ? $application->icon : md5($contents) . '.' . $extension); // Private apps could have here duplicated icons folder if (strpos($path, 'icons/icons/') !== false) { @@ -340,7 +338,7 @@ class ItemController extends Controller public function destroy(Request $request, int $id): RedirectResponse { // - $force = (bool) $request->input('force'); + $force = (bool)$request->input('force'); if ($force) { Item::withTrashed() ->where('id', $id) @@ -394,11 +392,11 @@ class ItemController extends Controller $output['custom'] = null; $app = Application::single($appid); - $output = (array) $app; + $output = (array)$app; $appdetails = Application::getApp($appid); - if ((bool) $app->enhanced === true) { + if ((bool)$app->enhanced === true) { // if(!isset($app->config)) { // class based config $output['custom'] = className($appdetails->name) . '.config'; // } @@ -444,7 +442,7 @@ class ItemController extends Controller } $app_details = new $app(); - $app_details->config = (object) $data; + $app_details->config = (object)$data; $app_details->test(); } diff --git a/tests/Unit/helpers/IsImageTest.php b/tests/Unit/helpers/IsImageTest.php index 0b3d16bc..8b14a794 100644 --- a/tests/Unit/helpers/IsImageTest.php +++ b/tests/Unit/helpers/IsImageTest.php @@ -9,9 +9,21 @@ class IsImageTest extends TestCase /** * @return void */ - public function test_isImage_returns_false_when_file_is_not_image() + public function test_returns_true_when_file_is_image() { - $actual = isImage(""); + $file = file_get_contents(__DIR__ . '/fixtures/heimdall-icon-small.png'); + + $actual = isImage($file, 'png'); + + $this->assertTrue($actual); + } + + /** + * @return void + */ + public function test_returns_false_when_file_extension_is_image_but_content_is_not() + { + $actual = isImage("", "png"); $this->assertFalse($actual); } @@ -19,24 +31,12 @@ class IsImageTest extends TestCase /** * @return void */ - public function test_isImage_returns_true_when_file_is_image() + public function test_returns_false_when_file_extension_is_not_image_but_content_is() { $file = file_get_contents(__DIR__ . '/fixtures/heimdall-icon-small.png'); - $actual = isImage($file); + $actual = isImage($file, 'php'); - $this->assertTrue($actual); - } - - /** - * @return void - */ - public function test_isImage_returns_false_when_file_is_php_but_png() - { - $file = file_get_contents(__DIR__ . '/fixtures/heimdall-icon-small-php.php'); - - $actual = isImage($file); - - $this->assertTrue($actual); + $this->assertFalse($actual); } } diff --git a/tests/Unit/helpers/fixtures/heimdall-icon-small-php.php b/tests/Unit/helpers/fixtures/heimdall-icon-small-php.php deleted file mode 100644 index 107f9cd5e3e72bd4d3275229600638fd40382763..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 5362 zcmchb^-~m%)5q^aqXNr-v*iJ{u~#9!ls%CNHKlK30$x)C0Hg)# zUPJ3_}12sX^! zIucadwYErhjp5|D8t9UlRz1rm)9`qQoo1F}a90jilq3s}S0?Ltt$1K~T4D4z#8tk; zXE83aKZ`4*JANy^kzdaN)O-dsm~M;Tt>s?%QOyeOZX*w|G>A5w2?O7Yz~VdU8Rr#JB7FhV z7V1K30qv5?p@Gm`wQ<|>zG>bbW=Y)o986H!OGu&pL@(*Gtz6`AvJ_vuXV94@(FBgC z_8emD;sov3w=aYRi9iKii+HXCLpcX@-hshXnWCEH%1-vjyhJ%nAbc>~<`_sz=b-$eHiN){HZenBiqE*tP8Fq6L) zFx$d%prNLyh3r@}c>>0sXD9!P{OMqWxnnmNX>Q6rlw4 z=-iacU~E&|v-FsD%a}qLPa8Eb&k?5=6kwtYgIZy{%T|+toZJvs|9q!ocSjJU_G{|e z(OA;iZXpc3nIaWZFSqLBi|;r>l$qpmW3#n#-l+y?qO`Hh`*nSXxw_Dli6c;lO^?p& z4=}z*TnT&Z7?eC?Z_#W#oXMA4BD8S8StVrrVgxG-$pMH6et;j!ATYk12u|LQcIIIU zs_x<2CQxYZT`33Mz_#d|ikM8ug7^RAm(#C(|9kaXSPC>xB5VYn^#Y8>SKo#D1W-_M z4@QGnX@V-nCHQX$1WlwxLfy6&m8Pr9BWj2V251*j=;;uOxnR&FK*(PE>xq zbb>1UO5NM|q2w+asd}3N4hx2Q`OQ_cRL|YtyYbCeCc6yGUrVgpZ2y>(d!eF($3yRl z;|wu;t9vOFh=G%=nxAD@8d6t#+AbNY^r*Jjf|jZF5#Z=~*qV9jJalpyj1SXp2o5i0 zD_ibwXEhcf*wh-?uW$}$;bzUWPRTiSjXIk$!$NwphcvgmDD1dTha`c1|~JMF!~>=;Y5 zRFDUjiUC5`OT$zhbG`NkqIfTQjN zoY{XK!3q3OkP7ySYbye59QM$n{bVO~8NjPUu9-X1v_PDzsIUzD2ebEpHG$U^jd=Pqq;dt+tO3UcG{qYA>z?Lk{!L- zHa-$hc1?T^oE3DR8{ZnzfdCcsCDLI*o8AW}EaV^}X=nVFP;u!U`i)~mdDgZgYUcVJ5UTPw}^<>!OO**L+OMDe`9~{>qONpWld2~Fdcl(_! zkTv}f9;{1_zCCb8``F4 z%L98pe;6>#jqrPx!codbzTLN@Z^*n~7epOSYR7e~0$*0CR= zrjPu{4RZ(8%xggm9TWh%HAvS_aWkxKb zk~IS6?w;7lc-kQbbz9uD7k{q}L9aQl2t5cS3l}u&p9vJClr^J=t>ox_76l?cU3JX| z^9zSK>ZX0Vxgb%?_JLaV5mE?T+vG%jZc)bAxN-OS6P`dx%2}Y@pWjxv>UUQ~Lcm5w zuq(RnbN_Vx&~u`Z;vn{eq6w0b%EboIl?%+r4_y&8`&bTx zJQ>Md?9^Og>*LS6Kxl1<`p$Dwspvzb(6O_dVMf&?WBa@4= z{xmt5*~89Ht?Ys*D^5Be&U?^{ArPMQ{Q|4Yw=!+l$$L36r5tze^!C`FlQ)go2iM?c z-nF=sR~(j?!bj5w>0^Qg#r!=;c`b9WX*%loRLa*8XF*!7^95x}>@f7PM7*6Y}%(M-*f#|xSb`35Znb>vUo-qI{lxKqm3;?~4)$AgYz{z1g8 zqs^#s8};#U7Mn;3ofs6LRIA2c)UV8@eQ>%yK5c3)bdNJ)O)QtbC{lQ87(j(J*A+3Z zJ(*U-QZD2~8Zlp3Fe3AGsU&1`7}e9tnsBLTL51$%vCD(8HiNnpA8eJFUsNRlKjrd% zQkn6DRoe#@^vvjw*4{$}zI6ynd78wUDjAAU=0v$aX$WAXF?k)W7>(9)KGDC>x&o@sQNo;!VA3o0q41b5bVr>#lzM0Wnu#&->R*fHVvRBkt zcudu*!};;3cEXxJG-O$aWU8mx_%cZ`gSnu^~QS3*k`;TDJvOjakS z?!kwKn{~S%MP9^+_ETwX2pjfE>E2y8>*BzfC)l$ehVw(sNv>F;8^bccF#nl7D82YW z=(xi&3q~rj5#-y}_SG$J^Cd;2WMa(A2)*k8xpSIf>bd8}ke zoJ_@8PgO$EcQ3@r!FFn95cuO;b?VxV!w zJzO4jEOJWcq%=c+8jC4RI&9h#HT3y6j~9xsXlKPk(!6sjMttqB!)j|zwlyc@zn%kB zn?8(G!hP1^6>=TAC zw=)DQB(GjgUO5w zSuwcy1~~D)ng$bF%{q?1*-ysms4|MMe%q|9b(9L4-Q1pd&f-~^UPm^)V#8j5sW-&K;mB(Xag%wf$h$FTigEU z>ll_3X`|~ke~RHsa(Dl&MC2aZkP(ecf~LflR4H&z*r_~#q!y*<93fMS{{+6gddsA9 z5!l-!-#Pi*fU1!56&t4(l&~;J(U``|(swofYcz+jrg~+@Xw$xZb%$J*8m{oJ%rUoP z97?~}uVM8QBkE)5#@OB>yp~t!7cu_5$Km=K^s}@>Vfns(j^tE6W{xF{vnad1Z8c2;~y|Hfjjl*dnoE=97jz; zky7KFIu^Yg*B7zfOyf=^ui6say;q}uvtm5K> zTP*!$Ur-m5*DM2_vFoKu3M@Z<&+6A7q0guo&q&LFy~gf7It%{%(&n^B@7GZ)7OWnJ zXK1bX8RE@BCd0comST{p!T-jb>W~FBU(3Q}k9Y+nR8eItU+1qUUzA%26{ z5iA_iz_Bo6TjI_xVPY?|SeT~O5Z%e_YSFjUM&?Ltk#ba4H?ht9$gR#ws83ld_apbC zref(5=YMU(#<;6QFsI&k8D*+GyvbgdByarH|J}PYe|kdHT**bg$TKOQw}X6IS*E$j zdOB$XlvJO`&!&++Z9NJ5#7M6sRJ)3F>+y_sb4wi^OArFrmfE+m(!2pFXO~P#h?GOs zjwwRyvjUG>XgpUA5ww!$1lAH0h$3nUA}=U>tl|5dOr2gM{Oq`WKBM4~u3@3-U(Y-y4C~N1_T*9~bli~ZfmV6h zyO{Y;!m8I|cDnnm4%5cCfh`*@P57*mtg5E{-8xi0v+_&K4j~Q3#bB3BMPJ?p{%5rE z?%1Z1f+fS|P0!^49$tpd`v%dNa=kD!%NYk6Z4>nDp#Up!vu5)li%2#HIqe|VZL)8+D11Xa#2+(Ha(RoVeeJ+77 ziBj{f!bA@Wx%h?qi|y6e0=@0jYLL&Jna%eIUbZFIk=}nJC1Y%vfyqmt}jmg zRa{FlU}@@lo_fBci~WoGvGS5ue3XUycCy<)?Wk;0Kw}~`j?T7_Z*j}2UNAPSUaF+Za_Iz$UgA#X-Bo|ZzS`PF zG90UaYc!(8uB|~~$`P;e^Fh~y^AfT_e6hYAnVf6!;$+Sp_T|rRJXYnh&~4oB0%N