Re: [PATCH] connscheduler.cgi: Remove cleanhtml command from Remark

[Adolf Belka <adolf.belka@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 5107 bytes --]

Hi Michael,

On 06/03/2024 22:28, Michael Tremer wrote:
> Hello Adolf,
> 
> 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 
to german umlauts being changed. The acceptance of that patch is what 
made me create these patches as they all had the same problem with 
remarks as well. If this can't be accepted as is then that patch needs 
to be reverted.

https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=7c6ff5ff12331a53f416080a44c8d6145e78bfac
> 
> 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 will 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’t be interpreted by the browser.
> 
> We might have problems where we accidentally call the cleanhtml function twice 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 “äöü”?
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 
server entry as an example
Ä ã ö â á à

and the remark was changed to
Ä ã ö â á Ã

and if I edit the entry but don't change the remark the new characters 
above get changed again into
Ä ã ö â á ÃÂ


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

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

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

Regards,
Adolf.

> 
> -Michael
> 
>> 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 marks such as
>>    umlauts or grave accents etc get encoded into other characters.
>> - If Freifunk München e.V. is entered as a remark it gets converted to
>>    Freifunk München e.V.
>> - cleanhtml is only removed from Remarks or Comment fields. In other places 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.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{'ACTION'} eq 'update') )
>>    $CONNSCHED::config[$i]{'DAYSTYPE'} = lc($cgiparams{'ACTION_DAYSTYPE'});
>>    $CONNSCHED::config[$i]{'DAYS'} = $l_days;
>>    $CONNSCHED::config[$i]{'WEEKDAYS'} = $l_weekdays;
>> -  $CONNSCHED::config[$i]{'COMMENT'} = &Header::cleanhtml($cgiparams{'ACTION_COMMENT'});
>> +  $CONNSCHED::config[$i]{'COMMENT'} = $cgiparams{'ACTION_COMMENT'};
>>
>>    &CONNSCHED::WriteConfig;
>> }
>> -- 
>> 2.44.0
>>
> 

-- 
Sent from my laptop