Welcome Guest, Not a member yet? Register   Sign In
Insecure Form_validation rule "encode_php_tags"
#1

[eluser]Phil Sturgeon[/eluser]
At the moment if I were to enter <?pHp, <?Php, <?phP etc then it would get through.

Current code:
Code:
function encode_php_tags($str)
    {
        return str_replace(array('<?php', '<?PHP', '<?', '?>'),  array('<?php', '<?PHP', '<?', '?>'), $str);
    }

Should be:

Code:
function encode_php_tags($str)
    {
        return str_ireplace(array('<?php', '<?', '?>'),  array('<?php', '<?', '?>'), $str);
    }

str_ireplace is case-sensitive so all combinations of upper/lower-case will be matched.
#2

[eluser]Tim Brownlaw[/eluser]
Sorry for sounding "anal" Phil but I think you meant to say

"str_ireplace is NOT case-sensitive........."

Good pickup by the way...

The function "encode_php_tags" occurs in 3 places
helpers/security_helper.php
libraries/Form_validation.php and
libraries/Validation.php

Cheers
Tim
#3

[eluser]Derek Jones[/eluser]
What's the security issue, Phil? <?pHp is going to be converted to <pHp. Also, str_ireplace() is PHP5 only. It does seem silly to replace the 'php' and 'PHP' at all, though.
#4

[eluser]Phil Sturgeon[/eluser]
The security issue is that this function is suppored to encode all PHP tags and it does not. If str_ireplace is out of the question the a simple regular expression would achieve the same goal.

It may be silly and I have never used this validation method, but this just doesn't do what it says on the tin. :-)

And yea Tim, I said the wrong thing... oops. >.<
#5

[eluser]Derek Jones[/eluser]
Phil, you still are not describing a security issue. The PHP tags are all going to be encoded by the 3rd and 4th elements of the find/replace arrays. The first two are redundant and unnecessary. It doesn't need to cue off of any letters at all, case-sensitive or otherwise.
#6

[eluser]Phil Sturgeon[/eluser]
Touché sir, touché. I made the observation late at night and didn't think to re-check when I was more awake. :red:
#7

[eluser]Derek Jones[/eluser]
I'm still glad you pointed it out, afterall, the first two array elements are silly. :-D




Theme © iAndrew 2016 - Forum software by © MyBB