0008743: function string_contains_scripting_chars should check for ' and " - MantisBT - Signup temporarily disabled due to spam

ID	Project	Category	View Status	Date Submitted	Last Update

0008743	mantisbt	security	public	2008-01-13 14:13	2011-08-05 02:25

Reporter	schoenfeld	Assigned To	dhx
Priority	normal	Severity	tweak	Reproducibility	have not tried
Status	closed	Resolution	won't fix

Summary	0008743: function string_contains_scripting_chars should check for ' and "
Description	The function string_contains_scripting_chars in core/string_api.php is only checking for the ocurrence of '>' and '<'. To avoid XSS attacks this is inadequate under certain cirumstances. Such circumstances are not yet in the mantis code, but they could become true in the future, therefore the function should be enhanced to also check for the ocurrence of ' and ".
Tags	No tags attached.
Attached Files	8743.patch (755 bytes) diff -u -urN mantis-1.1.0.orig/core/string_api.php mantis-1.1.0/core/string_api.php --- mantis-1.1.0.orig/core/string_api.php 2008-01-13 20:07:45.270746335 +0100 +++ mantis-1.1.0/core/string_api.php 2008-01-13 20:13:04.752952588 +0100 @@ -716,7 +716,8 @@ # -------------------- # Checks the supplied string for scripting characters, if it contains any, then return true, otherwise return false. function string_contains_scripting_chars( $p_string ) { - if ( ( strstr( $p_string, '<' ) !== false ) \|\| ( strstr( $p_string, '>' ) !== false ) ) { + if ( ( strstr( $p_string, '<' ) !== false ) \|\| ( strstr( $p_string, '>' ) !== false ) \|\| + ( strstr( $p_string, '\'') !== false ) \|\| ( strstr( $p_string, '"' ) !== false ) ) return true; } 8743.patch (755 bytes)

schoenfeld 2008-01-13 14:14 reporter ~0016658	The attached patch hopefully fixes the issue. However: It is untested.

giallu 2008-01-13 18:34 reporter ~0016661	Ouch, I think we should just get rid of that ugly function... I checked and it used 3 times in the whole code: in custom_field_create, triggers an error if fails on the field name in custom_field_update, the same in user_is_realname_valid, prevents real names with those chars Now, your patch breaks 3, as surnames like O'Grady will become invalid, but the main point is that I fail to see why we should prevent users real names or custom fields from containing > and < Of course, provided we ENSURE all output is properly escaped to avoid XSS...

dhx 2009-06-27 13:23 reporter ~0022290 Last edited: 2009-06-27 13:24	Agreed that this is a fairly useless function. We should be replacing > and < with safe output such as &gt and &lt instead. Imagine semi-colons at the end there :)

dhx 2010-09-18 01:22 reporter ~0026766	Won't fix as per removal of input side real name validation in 0012368.