From mboxrd@z Thu Jan  1 00:00:00 1970
From: Adolf Belka <adolf.belka@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] connscheduler.cgi: Remove cleanhtml command from Remark
Date: Wed, 06 Mar 2024 23:23:45 +0100
Message-ID: <55e772fb-9252-438a-a7a3-af634ac16426@ipfire.org>
In-Reply-To: <666E110C-A2EB-4BAE-9D93-44E80DBD4C00@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============0427450628972917693=="
List-Id: <development.lists.ipfire.org>

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

Hi Michael,

On 06/03/2024 22:28, Michael Tremer wrote:
> Hello Adolf,
>=20
> I believe that I cannot merge these patches.
Then you need to also look back at the dns.cgi patch for the bug fix due=20
to german umlauts being changed. The acceptance of that patch is what=20
made me create these patches as they all had the same problem with=20
remarks as well. If this can't be accepted as is then that patch needs=20
to be reverted.

https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dcommit;h=3D7c6ff5ff12331a53f41=
6080a44c8d6145e78bfac
>=20
> The reason simply is that it would create a store cross-site scripting atta=
ck vector because someone could store some <script> tags with some JS which w=
ill be executed if another admin opens the same page.
>=20
> That is why we escape the content so that if there are any special characte=
rs like <> for HTML tags they won=E2=80=99t be interpreted by the browser.
>=20
> We might have problems where we accidentally call the cleanhtml function tw=
ice which should show garbage. We might also have a problem where the functio=
n is not giving us the output that we want.
>=20
> Which strings have been causing problems? Just German umlauts like =E2=80=
=9C=C3=A4=C3=B6=C3=BC=E2=80=9D?
It is any character that has an accent or other diacritical mark.

I just tried entering the following into the remark section for a dns=20
server entry as an example
=C3=84 =C3=A3 =C3=B6 =C3=A2 =C3=A1 =C3=A0

and the remark was changed to
=C3=83=E2=80=9E =C3=83=C2=A3 =C3=83=C2=B6 =C3=83=C2=A2 =C3=83=C2=A1 =C3=83

and if I edit the entry but don't change the remark the new characters=20
above get changed again into
=C3=83=C6=92=C3=A2=E2=82=AC=C5=BE =C3=83=C6=92=C3=82=C2=A3 =C3=83=C6=92=C3=82=
=C2=B6 =C3=83=C6=92=C3=82=C2=A2 =C3=83=C6=92=C3=82=C2=A1 =C3=83=C6=92=C3=82


I would have expected that running cleanhtml should result in characters=20
that are considered safe after one run through but it seems that the=20
encoding creates characters that are encoded again by cleanhtml as being=20
unsafe and then those ones are again still considered unsafe.

If the cleanhtml command needs to stay being used also for the remark=20
entries then I have no idea how to allow german umlauts and other=20
accented characters to be shown correctly because they are all higher=20
bit ascii characters and those are encoded by default by the cleanhtml=20
process as being considered unsafe so I would either need some help on=20
how to deal with it or maybe someone else needs to pick up the original=20
bug#12395

The only thing I found is that cleanhtml calls the escape function which=20
calls the HTML::Entities::encode_entities command but that command can=20
have an additional option which defines the characters that are=20
considered unsafe, but then I would need some guidance on which=20
characters are considered unsafe and if that set applies to all=20
invocations of the cleanhtml command. ie should a modified cleanhtml=20
command with the extra option for the HTML::Entities::encode_entities=20
command only be used for some of the cleanhtml calls and if so would=20
that be only the remark/comment entries?

Regards,
Adolf.

>=20
> -Michael
>=20
>> On 5 Mar 2024, at 19:44, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>>
>> - Using cleanhtml on Remarks means that all characters with diacritical ma=
rks 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 place=
s it has been
>>    left in place.
>>
>> 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(-)
>>
>> diff --git a/html/cgi-bin/connscheduler.cgi b/html/cgi-bin/connscheduler.c=
gi
>> 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{'A=
CTION'} 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{'AC=
TION_COMMENT'});
>> +  $CONNSCHED::config[$i]{'COMMENT'} =3D $cgiparams{'ACTION_COMMENT'};
>>
>>    &CONNSCHED::WriteConfig;
>> }
>> --=20
>> 2.44.0
>>
>=20

--=20
Sent from my laptop

--===============0427450628972917693==--