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

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=7c6ff5ff12331a53f416080a...

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@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@ipfire.org Signed-off-by: Adolf Belka adolf.belka@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@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