Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom test to demonstrate everything breaks #15

Open
wants to merge 3 commits into
base: 1.x-dev
Choose a base branch
from

Conversation

wilsonge
Copy link
Contributor

So if you run a string containing a symbol < through the filter class then you find that it gets stripped.

It's something inside the tag cleaning - but obviously we don't want all < tags to be stripped

@wilsonge wilsonge changed the title Custom test to demonstrate something Custom test to demonstrate everything breaks Nov 11, 2015
@C-Lodder
Copy link

C-Lodder commented Nov 11, 2015

It all starts here: https://github.com/joomla-framework/filter/blob/master/src/InputFilter.php#L335

Just a possible solution off the top of my head:

  1. Check for <
  2. Match the next string against the $tagBlacklist array values
  3. If there's a match, continue to filter as it's currently doing
  4. Else convert < to its html entity and skip the filtering.

If this workaround it acceptable, let me know and I'll be more than happy to do it.

@photodude
Copy link
Contributor

@wilsonge
It's probably the overly broad assumption that there is a tag if there is a < in the tag cleaning method
https://github.com/joomla-framework/filter/blob/master/src/InputFilter.php#L553-L554

@mbabker
Copy link
Contributor

mbabker commented Mar 2, 2016

@wilsonge You doing anything with this?

@wilsonge
Copy link
Contributor Author

wilsonge commented Mar 2, 2016

I can't find a way to "make this work" :( But there's definitely an issue that needs solving as the unit test demonstrates

@photodude
Copy link
Contributor

Could we utilize HTML parsing with the PHP DOM module, for this part of the Filter to bypass the overly broad assumption that there is a tag if there is a < in the tag cleaning method?

@mbabker
Copy link
Contributor

mbabker commented Mar 23, 2016

Possibly? Still should account for what'll probably be rare cases where PHP is configured with --disable-dom though.

@csthomas
Copy link
Contributor

I can add such improvement direct to joomla after joomla/joomla-cms#16201 will be merged.
I did a test on my joomla and I got a lots of errors in other tests.
Are all changes accepted? (see below)

There were 30 failures:

1) JFilterInputTest::testCleanByCallingMember with data set "tag_01" ('', '<em', 'em', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'em'
+'&lt;em'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:803

2) JFilterInputTest::testCleanByCallingMember with data set "Malformed Nested tags" ('', '<em><strongFred</strong></em>', 'strongFred', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'strongFred'
+'&lt;strongFred'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:803

3) JFilterInputTest::testCleanByCallingMember with data set "missing_quote" ('string', '<img height="123 />', 'img height="123 /&gt;"', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img height="123 /&gt;"'
+'&lt;img height="123 /&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:803

4) JFilterInputTest::testCleanWithImgWhitelisted with data set "tag_01" ('', '<em', 'em', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'em'
+'&lt;em'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:951

5) JFilterInputTest::testCleanWithImgWhitelisted with data set "Malformed Nested tags" ('', '<em><strongFred</strong></em>', 'strongFred', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'strongFred'
+'&lt;strongFred'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:951

6) JFilterInputTest::testCleanWithImgWhitelisted with data set "missing_quote" ('string', '<img height="123 />', 'img height="123 /&gt;"', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img height="123 /&gt;"'
+'&lt;img height="123 /&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:951

7) JFilterInputTest::testCleanWithImgWhitelisted with data set "hanging_quote2" ('string', '<img src slkdjls " this is "m... stuff', 'img src slkdjls " this is "mo... stuff', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img src slkdjls " this is "more " stuff'
+'&lt;img src slkdjls " this is "more " stuff'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:951

8) JFilterInputTest::testCleanWithClassWhitelisted with data set "tag_01" ('', '<em', 'em', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'em'
+'&lt;em'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1065

9) JFilterInputTest::testCleanWithClassWhitelisted with data set "Malformed Nested tags" ('', '<em><strongFred</strong></em>', 'strongFred', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'strongFred'
+'&lt;strongFred'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1065

10) JFilterInputTest::testCleanWithClassWhitelisted with data set "missing_quote" ('string', '<img height="123 />', 'img height="123 /&gt;"', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img height="123 /&gt;"'
+'&lt;img height="123 /&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1065

11) JFilterInputTest::testCleanWithImgAndClassWhitelisted with data set "tag_01" ('', '<em', 'em', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'em'
+'&lt;em'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1186

12) JFilterInputTest::testCleanWithImgAndClassWhitelisted with data set "Malformed Nested tags" ('', '<em><strongFred</strong></em>', 'strongFred', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'strongFred'
+'&lt;strongFred'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1186

13) JFilterInputTest::testCleanWithImgAndClassWhitelisted with data set "missing_quote" ('string', '<img height="123 />', 'img height="123 /&gt;"', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img height="123 /&gt;"'
+'&lt;img height="123 /&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1186

14) JFilterInputTest::testCleanWithDefaultBlackList with data set "tag_01" ('', '<em', 'em', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'em'
+'&lt;em'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

15) JFilterInputTest::testCleanWithDefaultBlackList with data set "Malformed Nested tags" ('', '<em><strongFred</strong></em>', '<em>strongFred</strong></em>', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<em>strongFred</strong></em>'
+'<em>&lt;strongFred</strong></em>'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

16) JFilterInputTest::testCleanWithDefaultBlackList with data set "missing_quote" ('string', '<img height="123 />', 'img height="123 /&gt;"', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img height="123 /&gt;"'
+'&lt;img height="123 /&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

17) JFilterInputTest::testCleanWithDefaultBlackList with data set "security_tracker_24802_b" ('', '<img src="<img src=x"/onerror...)"//>"', 'img src="&lt;img src=x&quot;/.../&gt;"', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img src="&lt;img src=x&quot;/onerror=alert(1)&quot;//&gt;"'
+'&lt;img src="&lt;img src=x&quot;/onerror=alert(1)&quot;//&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

18) JFilterInputTest::testCleanWithDefaultBlackList with data set "security_tracker_24802_c" ('', '<img src="<img src=x"/onerror...1)"//>', 'img src="&lt;img src=x&quot;/.../&gt;"', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img src="&lt;img src=x&quot;/onerror=alert(1)&quot;//&gt;"'
+'&lt;img src="&lt;img src=x&quot;/onerror=alert(1)&quot;//&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

19) JFilterInputTest::testCleanWithDefaultBlackList with data set "security_tracker_24802_e" ('', '<img src=<img src=x"/onerror=...1)//">', 'img src=<img src="x/onerror=a...//" />', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img src=<img src="x/onerror=alert(1)//" />'
+'&lt;img src=<img src="x/onerror=alert(1)//" />'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

20) JFilterInputTest::testCleanWithDefaultBlackList with data set "hanging_quote2" ('string', '<img src slkdjls " this is "m... stuff', 'img src slkdjls " this is "mo... stuff', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img src slkdjls " this is "more " stuff'
+'&lt;img src slkdjls " this is "more " stuff'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

21) JFilterInputTest::testCleanWithDefaultBlackList with data set "hanging_quote3" ('string', '<img src="\' />', 'img src="\' /&gt;"', 'From specific cases')
From specific cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img src="\' /&gt;"'
+'&lt;img src="\' /&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

22) JFilterInputTest::testCleanWithDefaultBlackList with data set "tracker25558a" ('string', '<SCRIPT SRC=http://jeffchanne...#<B />', 'SCRIPT SRC=http://jeffchannel...#<B />', 'Test mal-formed element from 25558a')
Test mal-formed element from 25558a
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'SCRIPT SRC=http://jeffchannell.com/evil.js#<B />'
+'&lt;SCRIPT SRC=http://jeffchannell.com/evil.js#<B />'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

23) JFilterInputTest::testCleanWithDefaultBlackList with data set "tracker25558e" ('string', '<b><script<b></b><alert(1)</s...t </b>', '<b>script<b></b>alert(1)/script </b>', 'Test mal-formed element from 25558e')
Test mal-formed element from 25558e
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<b>script<b></b>alert(1)/script </b>'
+'<b>&lt;script<b></b>&lt;alert(1)&lt;/script </b>'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1422

24) JFilterInputTest::testCleanWithImgBlackList with data set "tag_01" ('', '<em', 'em', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'em'
+'&lt;em'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1523

25) JFilterInputTest::testCleanWithImgBlackList with data set "Malformed Nested tags" ('', '<em><strongFred</strong></em>', '<em>strongFred</strong></em>', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<em>strongFred</strong></em>'
+'<em>&lt;strongFred</strong></em>'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1523

26) JFilterInputTest::testCleanWithImgBlackList with data set "missing_quote" ('string', '<img height="123 />', 'img height="123 /&gt;"', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img height="123 /&gt;"'
+'&lt;img height="123 /&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1523

27) JFilterInputTest::testCleanWithClassBlackList with data set "tag_01" ('', '<em', 'em', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'em'
+'&lt;em'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1744

28) JFilterInputTest::testCleanWithClassBlackList with data set "Malformed Nested tags" ('', '<em><strongFred</strong></em>', '<em>strongFred</strong></em>', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<em>strongFred</strong></em>'
+'<em>&lt;strongFred</strong></em>'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1744

29) JFilterInputTest::testCleanWithClassBlackList with data set "missing_quote" ('string', '<img height="123 />', 'img height="123 /&gt;"', 'From generic cases')
From generic cases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'img height="123 /&gt;"'
+'&lt;img height="123 /&gt;"'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:1744

30) JFormTest::testFilterField
Line:474 <>" are always illegal in host names.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'http://onmouseover=alert(2);'
+'http://onmouseover=alert(2);&lt;&gt;'

/home/tomash/public_html/a4/tests/unit/suites/libraries/joomla/form/JFormTest.php:474

@nibra
Copy link
Contributor

nibra commented Jan 19, 2021

I'd expect from the string filter that
a) I get a string (an array of strings) as a result, whatever I feed into the filter
b) HTML entities are decoded
c) nothing else is changed
Dealing with tags should solely be up to the HTML filter.

Would problems arise, if that behaviour gets implemented into 1.x? I'd like to add it to 2.0 in any case.
@wilsonge ? @Hackwar ? @HLeithner ?

@Hackwar
Copy link
Contributor

Hackwar commented Aug 26, 2023

This is still a current issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants