From 551dff9e719f0256e198b25df2ea43f1cc5cf1e5 Mon Sep 17 00:00:00 2001
From: Alexander Todorov <atodorov@otb.bg>
Date: Tue, 18 Apr 2023 18:52:44 +0300
Subject: [PATCH] Enable additional upload validators for django-attachments

to block files which may be executed by the browser and cause an issue
with users who aren't careful.

Test with .exe file from React OS!
---
 docs/source/modules/tcms.kiwi_attachments.rst |  15 +++++++++
 .../tcms.kiwi_attachments.validators.rst      |   7 +++++
 docs/source/modules/tcms.rst                  |   1 +
 tcms/kiwi_attachments/__init__.py             |   0
 tcms/kiwi_attachments/apps.py                 |  14 +++++++++
 tcms/kiwi_attachments/tests/__init__.py       |   0
 .../kiwi_attachments/tests/test_validators.py |  29 ++++++++++++++++++
 tcms/kiwi_attachments/validators.py           |  23 ++++++++++++++
 tcms/settings/common.py                       |   2 +-
 tests/ui/data/reactos_csrss.exe               | Bin 0 -> 9728 bytes
 10 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 docs/source/modules/tcms.kiwi_attachments.rst
 create mode 100644 docs/source/modules/tcms.kiwi_attachments.validators.rst
 create mode 100644 tcms/kiwi_attachments/__init__.py
 create mode 100644 tcms/kiwi_attachments/apps.py
 create mode 100644 tcms/kiwi_attachments/tests/__init__.py
 create mode 100644 tcms/kiwi_attachments/tests/test_validators.py
 create mode 100644 tcms/kiwi_attachments/validators.py
 create mode 100755 tests/ui/data/reactos_csrss.exe

diff --git a/docs/source/modules/tcms.kiwi_attachments.rst b/docs/source/modules/tcms.kiwi_attachments.rst
new file mode 100644
index 0000000000..cf54db0f28
--- /dev/null
+++ b/docs/source/modules/tcms.kiwi_attachments.rst
@@ -0,0 +1,15 @@
+tcms.kiwi\_attachments package
+==============================
+
+.. automodule:: tcms.kiwi_attachments
+   :members:
+   :undoc-members:
+   :show-inheritance:
+
+Submodules
+----------
+
+.. toctree::
+   :maxdepth: 4
+
+   tcms.kiwi_attachments.validators
diff --git a/docs/source/modules/tcms.kiwi_attachments.validators.rst b/docs/source/modules/tcms.kiwi_attachments.validators.rst
new file mode 100644
index 0000000000..a457456bdc
--- /dev/null
+++ b/docs/source/modules/tcms.kiwi_attachments.validators.rst
@@ -0,0 +1,7 @@
+tcms.kiwi\_attachments.validators module
+========================================
+
+.. automodule:: tcms.kiwi_attachments.validators
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/source/modules/tcms.rst b/docs/source/modules/tcms.rst
index 64e66c6707..ea3b3f4760 100644
--- a/docs/source/modules/tcms.rst
+++ b/docs/source/modules/tcms.rst
@@ -15,6 +15,7 @@ Subpackages
    tcms.bugs
    tcms.core
    tcms.issuetracker
+   tcms.kiwi_attachments
    tcms.kiwi_auth
    tcms.management
    tcms.rpc
diff --git a/tcms/kiwi_attachments/__init__.py b/tcms/kiwi_attachments/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tcms/kiwi_attachments/apps.py b/tcms/kiwi_attachments/apps.py
new file mode 100644
index 0000000000..c3b3d23d6e
--- /dev/null
+++ b/tcms/kiwi_attachments/apps.py
@@ -0,0 +1,14 @@
+from attachments.apps import AttachmentsConfig
+
+from . import validators
+
+
+class AppConfig(AttachmentsConfig):
+    """
+    Defines custom form validators!
+    """
+
+    attachment_validators = (
+        validators.deny_uploads_ending_in_dot_exe,
+        validators.deny_uploads_containing_script_tag,
+    )
diff --git a/tcms/kiwi_attachments/tests/__init__.py b/tcms/kiwi_attachments/tests/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tcms/kiwi_attachments/tests/test_validators.py b/tcms/kiwi_attachments/tests/test_validators.py
new file mode 100644
index 0000000000..1f17135483
--- /dev/null
+++ b/tcms/kiwi_attachments/tests/test_validators.py
@@ -0,0 +1,29 @@
+# -*- coding: utf-8 -*-
+# pylint: disable=attribute-defined-outside-init, invalid-name, objects-update-used
+
+import base64
+from xmlrpc.client import Fault
+
+from tcms.rpc.tests.utils import APITestCase
+
+
+class TestValidators(APITestCase):
+    def test_uploading_svg_with_inline_script_should_fail(self):
+        with open("tests/ui/data/inline_javascript.svg", "rb") as svg_file:
+            b64 = base64.b64encode(svg_file.read()).decode()
+
+            with self.assertRaisesRegex(Fault, "File contains forbidden <script> tag"):
+                self.rpc_client.User.add_attachment("inline_javascript.svg", b64)
+
+    def test_uploading_filename_ending_in_dot_exe_should_fail(self):
+        with self.assertRaisesRegex(Fault, "Uploading executable files is forbidden"):
+            self.rpc_client.User.add_attachment("hello.exe", "a2l3aXRjbXM=")
+
+    def test_uploading_real_exe_file_should_fail(self):
+        with open("tests/ui/data/reactos_csrss.exe", "rb") as exe_file:
+            b64 = base64.b64encode(exe_file.read()).decode()
+
+            with self.assertRaisesRegex(
+                Fault, "Uploading executable files is forbidden"
+            ):
+                self.rpc_client.User.add_attachment("csrss.exe_from_reactos", b64)
diff --git a/tcms/kiwi_attachments/validators.py b/tcms/kiwi_attachments/validators.py
new file mode 100644
index 0000000000..3b8189219b
--- /dev/null
+++ b/tcms/kiwi_attachments/validators.py
@@ -0,0 +1,23 @@
+from django.forms import ValidationError
+from django.utils.translation import gettext_lazy as _
+
+
+def deny_uploads_containing_script_tag(uploaded_file):
+    for chunk in uploaded_file.chunks(2048):
+        if chunk.find(b"<script") > -1:
+            raise ValidationError(_("File contains forbidden <script> tag"))
+
+
+def deny_uploads_ending_in_dot_exe(uploaded_file):
+    message = _("Uploading executable files is forbidden")
+
+    if uploaded_file.name.find(".exe") > -1:
+        raise ValidationError(message)
+
+    if uploaded_file.content_type in [
+        "application/vnd.microsoft.portable-executable",
+        "application/x-dosexec",
+        "application/x-ms-dos-executable",
+        "application/x-msdownload",
+    ]:
+        raise ValidationError(message)
diff --git a/tcms/settings/common.py b/tcms/settings/common.py
index 031124f640..d1a10984ba 100644
--- a/tcms/settings/common.py
+++ b/tcms/settings/common.py
@@ -294,10 +294,10 @@
 TENANT_APPS = [
     "django.contrib.sites",
     "guardian",
-    "attachments",
     "django_comments",
     "modernrpc",
     "simple_history",
+    "tcms.kiwi_attachments.apps.AppConfig",
     "tcms.core.contrib.linkreference",
     "tcms.management",
     "tcms.testcases.apps.AppConfig",
diff --git a/tests/ui/data/reactos_csrss.exe b/tests/ui/data/reactos_csrss.exe
new file mode 100755
index 0000000000000000000000000000000000000000..446959397378985912db873e096279babc7ae80c
GIT binary patch
literal 9728
zcmeI14RBP|702(A&_)a-s8Oh;E^SayNy0}dQq(28TZj!w%qB=xe7x+wgh%!xzV{Y_
zpvG=u*@+qB*XUF`Q)eh@hw0dsQ53XMlt^coqNP)9X=Pe#lL_^sgIJXz{h#~Z+dX+i
zY^&o;XMD`v_kQ=Dd(OG%o}1lwSb1%~C=fyvqK%FUu?u&)0`l|pA6*!pa>3J6#LhDg
z%-rRxJutIroo@O&j8vPUCj3z~nM_&!7R_&@lYTwv57kHfiBwFhm^^t>xud!vEJUqu
zq8O>WA|{;9Bci}}rf<U6#g`ydWD5>W#l6&-MBdT!1R*90U#`i~4t!!UH~d&&gf71`
z4-=gE&P|SrDk1(wf&n4=DG;C0WkK)a)99+Ov`!1-FHFP-IYeuvO<<Q_RIRNrVydMI
z@hJJ|N5h)MBs6vfL{*?dx8>woU|Y%+Ga)=eZlJ<2jVPYwS_60>n^Vx(Wy_^Zb5lY*
zO};gto#iwpH=#&WGyLN5|1bj0*}r9uP8$rLAm0Z=<;9c_ec=<?P<auskWy%{uzZxR
zvaYIt$Q&)*zZbJ2nWN={;bW7>OgrXIqpWtO_0FJaqrb&O(vJbMBaby1p!{+TkW9}&
z=jz^YF%%W{hKsU=<wNhmrOeTy?B1S%lC4idd}ukGi9I|GkG+Aa%&tRpHcGnR$7lBX
z@<M1kDc#*F-7S)T(t6jN*uNJ_i^+Bnl4N_HKR~t{a2vWGR(b}~OZ&p_K<pe0my%lC
zPjMODH0@hHF)&)TMH-nlyaFc2(S1AV{x)=vY<I0Uj<vpxB&OTer#t?W`=e#uw*SM$
zj%v?Dsrm^1W{vD8!_q4(%JGhzA1!+TDu(}#aA3!-S%p1Y*t@2v=Z_`b=VQD#Sc>?L
zmfb7u6v_a1S7N}9tyBuNxJ#vob#GvroN<SoQJfvbmULf?8G~e1WHyI{HK{KcXw7U6
zh;%_;a4{(%S7<-`!b5WQL(c4dw4sa0k@V4?<0ajD$aHTR?Q6-C;i1gH1k93INXmPw
zPci?&VE7OXK~E2ML*`WmOdglb4A3Tpa2wh^L5OG16Jmhe{n7Y>-y^}hAvnCn7RYV;
zRgT6tuu;%h0m0F-ez}Iy;a!L=%;kh5V}<9B5#DAC7hOA!kLPh8Kd=S7g3`yg;NI}7
zjsSgyu+^q%5eFbZ3*9-sj8yU%DY*v%-LIr)lss{6_bb*_T_5^yotoL_v++&fE$@!J
zo_Ve?yX-_Je2Ds~k3=#9zO4<TcE_!6@55h=fNx}1*4>Z3Wrf-B!9m!-wnbKC5B9v0
zzB;qHSd{e4Kxc1|j(2fy_#o|Xb_<>KLv+%A^3mvMc3;+#XL!sR=+ll}cZ0I#`9guL
zXw7t<6zSuwnZ+XgPPX|Cbj>-~nhhU8`-_`!ec>Z^&b{i*xy~t>n-2?XWpDU!Z}Y*C
zTWH1(J63Mi$T;?(kK!12j-N+H#!-dL@ON#2Tn;?$D033#E}jD;+065W+3FLBU{~j;
zNWb5OMI0T(=0nkI$d93O-KYN1S=*3h{kV5Jcgw!R9?4UL=>BQ0O4Ha-*^<`dv6hro
zxiMw5YlcEUB`T|_Q&}Di*6K;6bMYcus?toin43&XOPG~W)1YR`C)2Em!jajt%q6p8
zbNs<bV<fV~A55j=F+cqbG*sQx{5ACrwU_!MmTINVCH|_;SyhYUos)92LM?3#hMu%$
z&+)gadR&Xmq@Ow-oy|$LC9e6c6n<0cmafM2n>4?v85=ZX9Mzcr->)&&UK!V0Dw9^F
zmb8pbN=FJ#VSNo+i)b)XQOz_PRQ$%(EX^=;{)HPGmtd{_gqE0@>l|-M?t<8a*==bk
zoij-+OPYE(xj{El$pnm7RHs{8HRIE4@oDESt;!D{o+kwTZjH0Pqv+elI4>>44)pIq
zdkO6@+HtgE$otXeqg{b^Et-W!*S*gDf1Ot<5X@>rQ`;M8Kg3jGnJ+>t@zq(4lwvDX
zBNjG{6hgKheG$#7Nw%hpglg%jq_cP82N-G6jD((4EzRu|TX}EOIv9$H7k!OZd|5o6
zib8UQrgn(m$sQ94XNfZlFj`02DG7OIm|`uB!MxL3V+IXmYgCPkUK(jm>QTH4MJ(id
zTT_a*-F_D0D<Wtb5t-06ZiY+uDTs*if7+5(EFP~w!<2I{kIt&G!Ra7fr(8byDuOTI
zuCYTt`7Q+&r}UqJp9U)+ygDGn!h-y)g#`-%t3*U7_za6iJk^MLQ3tNU=XdZ-*E5B0
zpQJZ6`3pmo<nC9>QF=Wol(rp|H~N<LL7ycIp^KzwgEn2ng$B(@(JE4qN=lh3u@J$!
z3~ixkkzos9g+394OhR-B74n<ljSAAF?nc~I5ryu0Sn%W3$AIj5^irP?onkiTyY@op
zH!(tfbYNVEY*Ok8!XF)@N%U4ilLna$^3#u4q-DP@t@<&VhIbKoVnSL&uWL67GVu>*
z39-rBT_rV{%wCnKz#1#WJn>C27hWzFi*S!YZiDpaQoLaLWPHei0sAUE%&$BOD~4lh
zt@s*bYa!Prr6^h*@Qzm5Cf3O~%tj37z|VPz?q$-}e8hLYSb)Ayis4ec!1@qhTOaKp
z`QpZu$li^-PZjW`6HP$(bj-DPC5E`suH;ui<74-)9-h;BRmj=7Lh;T=nu1d;V>?!-
zH^QG3-w^cpkm)LHxhu(Mqd;kcGQ3H8l0=;2jz@03oKg6=eFVs%&+#gStt0r7eFyE=
zcx&^4>3q2t@oUU8;)@~YUc_O>^!PLCbS~l)n@{{t##E<Rz!?s`guZgfoliQzOr7ML
zc?ZUun7?0g<Z1p_(Izv#0KAa#H^7U)FXK+~m!MtF_)>6)@mz3%@lx<zj052N7}tS!
zFs8qFI~iXCeu^=@7W~TL@8kIm{5^f%GyWUKYasJi#y5cf!8iz>IyJvO;*}n5XWWSK
zJ3aiUhxaiif2l6_2=4TfP4PI!_;T<G#-xApS$TPSS1Myn@i^aMcl}qg@zoe#&Uh_&
zC1VX7VZ08!ig7!54dVoOE#n)(H#0WCJ&ZSjw=%v7+|RfRe80o4{||b`_b|Q<^3O63
zfnQ`y;|CaLz{8BY!EZ69@#7vo@9ccM=|wignBspUW7^L?#@B)GW=!Kh^zd%R^!@m{
zhs8O0`*&izkZ~6LWe=ap_zsMhdiZP)pX=e7jPJ($at~j`_&$tZ!uSDjg@+e-c#(%M
z^YBs+U+Lix;|F1Hxrb{R(|M?4O!?8w_+jvM9#$D|!+4BwIk=VacJO+}17M5sUhqAP
zsmSvP<CnlcXFMCc)5E(NUx@MN7}MX|7Z_K82R*#s!v{S4GULZE|6RsE1;5Amaqvfs
ze*r$p_?KW`Sw3Hh3mESJU*zExj7h)2_z7^@!<~$u!uVzn-^Q5i-_H0M@Lh}tzz=~Z
z7U03Twt+un-;X=M57L*ebL|8#WAemLGTsBekj*Ei(rE7T3o(t23-Am!|IwL3{5~g)
z|F-}~m^|@(_I*t}<l*VeUmCxOG5v_IX8MR{vH8St<`2F1JjUkJ`|WlnPn==v>jc~7
zPJ0nsz^mDK7x;RnuN(X#v$qvIhw&ZYKQrzJ7cl$xf^TQz4}gy`ehB;?izmH{*Rk;(
z;6E{a^bUVEV|srNGyg>ZzkzuC!23P?2M>>UxSi=IeOo-dk@-jC+dbUx@n?^R-}LZq
z7H|3ixQ&kszqpV42ZK*@f4~R0JYKCk@KzquTGe#ieL2q+4OZfnT%5MVRhd9l2&JEx
zZnozOXQ{N+k+$Zp)RW7vnX4~Awai2fNl!-O>6lj8L9kIzqMWl%@PVjdRiX&dwsFCH
z2vaG5Ds2#|b3H?;%&^iOa@M#dDJ7xmNuel>&2>#RE5k}dB-|XTR~o~$^+Bb!CeoxR
zLeUc)kT_k@LDu{fldRDMCDc^kn47LxaWg-*4s{U)zR2=|q6!65I!r@Djl_`>3W^R&
zbR9}$QB=ru6Lk-h)S9T0D5+M`6>V3dsYF7T=En9Lnylv0goL_2Whl7~QBdPEQOA)k
zB6vz?JnxqxJM>dKv}6nua<!sdG?v*#9JUq(1)+o{lope%_cWmm*M<195Sb3PD%+tB
zuR|S*?T`!-tx->6jVY5aPHx!PY8TvA+cc#`H8p8di&8~PLFK7URtl-JW?7S~{B-&7
zapfgD2^{aF3#?4$6vUJu&kQN_vFa#{jy5|MQ5W5W{FvIA;)>@J@>H|TH@AeB-&$U=
z%k_?1jT@W3NKogv<nyH!mFTf?M)upqG(~k&#tE0F2I(a5*v+L~$}P-^&zSS(9tW9E
z>@?U>vGS9Z8p?@kqy)~ETbQRrkSEqrZJ!}Kd)WDjr_>l~{jG7eO`c7DiZRp8%e-1P
z*G}0}R5~#jzvlXh$H<fJ?w=Vq&{&i_;Fgm^;*0Auh``m*Q4!Nx(rpU<S|{5y(?uO+
jbN5+oX)&}7F2(hv=03O5|AM%HZ-CFhr7u)`juH4bLcA>w

literal 0
HcmV?d00001