From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] connscheduler.cgi: Remove cleanhtml command from Remark
Date: Wed, 06 Mar 2024 22:28:19 +0100
Message-ID: <666E110C-A2EB-4BAE-9D93-44E80DBD4C00@ipfire.org>
In-Reply-To: <20240305184418.3431741-1-adolf.belka@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3544516949836915967=="
List-Id: <development.lists.ipfire.org>

--===============3544516949836915967==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello Adolf,

I believe that I cannot merge these patches.

The reason simply is that it would create a store cross-site scripting attack=
 vector because someone could store some <script> tags with some JS which wil=
l be executed if another admin opens the same page.

That is why we escape the content so that if there are any special characters=
 like <> for HTML tags they won=E2=80=99t be interpreted by the browser.

We might have problems where we accidentally call the cleanhtml function twic=
e which should show garbage. We might also have a problem where the function =
is not giving us the output that we want.

Which strings have been causing problems? Just German umlauts like =E2=80=9C=
=C3=A4=C3=B6=C3=BC=E2=80=9D?

-Michael

> On 5 Mar 2024, at 19:44, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>=20
> - Using cleanhtml on Remarks means that all characters with diacritical mar=
ks such as
>   umlauts or grave accents etc get encoded into other characters.
> - If Freifunk M=C3=BCnchen e.V. is entered as a remark it gets converted to
>   Freifunk M=C3=83=C2=BCnchen e.V.
> - cleanhtml is only removed from Remarks or Comment fields. In other places=
 it has been
>   left in place.
>=20
> Tested-by: Adolf Belka <adolf.belka(a)ipfire.org>
> Signed-off-by: Adolf Belka <adolf.belka(a)ipfire.org>
> ---
> html/cgi-bin/connscheduler.cgi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>=20
> diff --git a/html/cgi-bin/connscheduler.cgi b/html/cgi-bin/connscheduler.cgi
> index cc78cbc1b..817247cc4 100644
> --- a/html/cgi-bin/connscheduler.cgi
> +++ b/html/cgi-bin/connscheduler.cgi
> @@ -2,7 +2,7 @@
> ###########################################################################=
####
> #                                                                          =
   #
> # IPFire.org - A linux based firewall                                      =
   #
> -# Copyright (C) 2007  Michael Tremer & Christian Schmidt                  =
    #
> +# Copyright (C) 2007-2024  IPFire Team  <info(a)ipfire.org>               =
      #
> #                                                                          =
   #
> # This program is free software: you can redistribute it and/or modify     =
   #
> # it under the terms of the GNU General Public License as published by     =
   #
> @@ -186,7 +186,7 @@ if ( ($cgiparams{'ACTION'} eq 'add') || ($cgiparams{'AC=
TION'} eq 'update') )
>   $CONNSCHED::config[$i]{'DAYSTYPE'} =3D lc($cgiparams{'ACTION_DAYSTYPE'});
>   $CONNSCHED::config[$i]{'DAYS'} =3D $l_days;
>   $CONNSCHED::config[$i]{'WEEKDAYS'} =3D $l_weekdays;
> -  $CONNSCHED::config[$i]{'COMMENT'} =3D &Header::cleanhtml($cgiparams{'ACT=
ION_COMMENT'});
> +  $CONNSCHED::config[$i]{'COMMENT'} =3D $cgiparams{'ACTION_COMMENT'};
>=20
>   &CONNSCHED::WriteConfig;
> }
> --=20
> 2.44.0
>=20


--===============3544516949836915967==--