Skip to content

Commit

Permalink
Issue #4081: do not replace '>' in style tags
Browse files Browse the repository at this point in the history
  • Loading branch information
bschmalhofer committed Jan 22, 2025
1 parent 098bc98 commit 37c4dcc
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 11 deletions.
16 changes: 14 additions & 2 deletions Kernel/cpan-lib/HTML/Scrubber.pm
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ If you're new to perl, good luck to you.
=cut

use 5.008; # enforce minimum perl version of 5.8
use v5.10; # enforce minimum perl version of 5.8
use strict;
use warnings;
use feature qw(state);

use HTML::Parser 3.47 ();
use HTML::Entities;
use Scalar::Util ('weaken');
Expand Down Expand Up @@ -471,6 +473,8 @@ sub _scrub_str {

my $s = $p->{"\0_s"};

state $last_start_tag = '';

# premptive handling of an event might turn off the rule based handling
if ( $s->{_preempt} && ref $s->{_preempt} eq 'CODE' ) {
if ( $e eq 'end' && $text eq '' && $s->{_ignore_empty_end} ) {
Expand All @@ -495,6 +499,7 @@ sub _scrub_str {
my $outstr = '';

if ( $e eq 'start' ) {
$last_start_tag = $t;
if ( exists $s->{_rules}->{$t} ) # is there a specific rule
{
if ( ref $s->{_rules}->{$t} ) # is it complicated?(not simple;)
Expand Down Expand Up @@ -548,7 +553,14 @@ sub _scrub_str {
}
elsif ( $e eq 'text' or $e eq 'default' ) {
$text =~ s/</&lt;/g; #https://rt.cpan.org/Public/Ticket/Attachment/83958/10332/scrubber.patch
$text =~ s/>/&gt;/g;

# This is very hackish.
if ( $last_start_tag eq 'style' ) {
# do not replace '>' in style tags
}
else {
$text =~ s/>/&gt;/g; # see https://rt.cpan.org/Public/Bug/Display.html?id=2991
}

$outstr .= $text;
}
Expand Down
20 changes: 11 additions & 9 deletions scripts/test/HTMLUtils/Safety.t
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use Test2::V0;

# OTOBO modules
use Kernel::System::UnitTest::RegisterOM; # set up $Kernel::OM
use Kernel::System::UnitTest::Diff qw(TextEqOrDiff);

# get HTMLUtils object
my $HTMLUtilsObject = $Kernel::OM->Get('Kernel::System::HTMLUtils');
Expand Down Expand Up @@ -662,17 +663,17 @@ END_INPUT
Result => {
Output => <<'END_OUTPUT',
<style type=" text/css">
div &gt; span {
div > span {
width: 200px;
}
</style>
<style type=" text/CSS ">
div &gt; span {
div > span {
width: expression( FormerlyEvilJS() );
}
</style>
<style type="text/css">
div &gt; span &gt; div {
div > span > div {
width: 200px;
}
</style>
Expand Down Expand Up @@ -933,7 +934,7 @@ for my $Test (@TestsWithDefaultConfig) {
else {
ok( !$Result{Replace}, 'not replaced', );
}
is( $Result{String}, $Test->{Result}->{Output}, 'output' );
TextEqOrDiff( $Result{String}, $Test->{Result}->{Output}, 'output' );
};
}

Expand Down Expand Up @@ -1166,7 +1167,7 @@ You should be able to continue reading these lessons, however.
Line => __LINE__,
},
{
Name => 'stype with remote background image protocol-relative URL, NoExtSrcLoad',
Name => 'style with remote background image protocol-relative URL, NoExtSrcLoad',
Input => '<a href="localhost" style="background-image:url(//localhost:8000/css-background)">localhost</a>',
Config => {
NoExtSrcLoad => 1,
Expand Down Expand Up @@ -1334,7 +1335,7 @@ for my $Test (@TestsWithExplicitConfig) {
else {
ok( !$Result{Replace}, 'not replaced', );
}
is( $Result{String}, $Test->{Result}->{Output}, 'output' );
TextEqOrDiff( $Result{String}, $Test->{Result}->{Output}, 'output' );
};
}

Expand Down Expand Up @@ -1461,9 +1462,10 @@ END_HTML
String => $String,
);

# all '>' in text elements are replaced by '&gt;'
my $ExpectedScrubbedString = ( $String =~ s/div > p/div &gt; p/r ) =~ s/greater: >/greater: &gt;/r;
is( $Result{String}, $ExpectedScrubbedString, 'greater sign encoded' );
# all '>' in text content, except style, are replaced by '&gt;'
my $ExpectedScrubbedString = $String =~ s/greater: >/greater: &gt;/r;

TextEqOrDiff( $Result{String}, $ExpectedScrubbedString, 'greater sign encoded' );
}

done_testing;

0 comments on commit 37c4dcc

Please sign in to comment.