1. Advertising
    y u no do it?

    Advertising (learn more)

    Advertise virtually anything here, with CPM banner ads, CPM email ads and CPC contextual links. You can target relevant areas of the site and show ads based on geographical location of the user if you wish.

    Starts at just $1 per CPM or $0.10 per CPC.

EVAL Security Issue

Discussion in 'PHP' started by Kalyse, Apr 19, 2007.

  1. #1
    I am letting users on a CMS enter PHP code to be parsed but I dont want some commands to be ran. Does anyone know a class or function that anyone has made to make my life easier.

    Im allowing them for the logical capabilities really. Btw, its 'their' site, they have the site hosted on my servers but I dont want them to be able to inject PHP code to mess with their files, because they dont get the source code, but it would be easy for them to get it if they were able to use certain functions.

    Infact, I think I just realised what I could do,

    preg match

    And anything that is like [a-zA-Z_]*\([.]+\) to remove automatically unless it is in an allowed function list.

    Can anyone think of ways this could be abused?
     
    Kalyse, Apr 19, 2007 IP
  2. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #2
    You could disable dangerous functions in php.ini


    
    disable_functions = dl,system,exec,passthru,shell_exec,unlink
    
    Code (markup):
    EDIT:

    This would completely disable them though. If you JUST want to disable them in your eval string, you'll need to replace them, in a way that it doesn't cause any parse errors. Maybe you can replace them with a dummy function that always returns true or something...
     
    nico_swd, Apr 19, 2007 IP
    Kalyse likes this.
  3. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #3
    No, I wouldnt replace, I just wouldnt let the user save the entry.

    THere is a submit form, all I would do is a match pattern and if its there then dont save the PHP code to the databse so it would never get loaded.
     
    Kalyse, Apr 19, 2007 IP
  4. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #4
    Do you know which functions you want to forbid? You can do a preg_match like this:
    
    
    
    if (preg_match('/(exec|unlink|stsyem|etc)/i', $user_submitted_code))
    {
         // Contains illegal function, handle error...
    }
    else
    {
         // Everything Okay
    }
    
    PHP:
     
    nico_swd, Apr 19, 2007 IP
  5. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #5
    No, its the other way around,

    I want to forbid EVERYTHING except,

    if(), time()
    and some of my own custom functions that I have created
     
    Kalyse, Apr 19, 2007 IP
  6. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #6
    Sorry for being slow, get you now. Give this a try: (Untested)

    
    
    $allowed_functions = array('if', 'elseif', 'time', 'array', 'echo', 'print', 'for', 'foreach', 'while');
    
    preg_match_all('/([\w]+)[\r\n\t\s]*\(/', $code, $functions);
    
    foreach ($functions[1] AS $function)
    {
         if (!in_array($function, $allowed_functions))
         {
              exit('Error: Call to unallowed function ' . $function);
         }
    }
    
    
    PHP:
    You'd need to specify everything that consists of alphanumeric chars followed by parenthesis, like arrays, etc...
     
    nico_swd, Apr 19, 2007 IP
  7. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #7
    Hey there is a problem.

    What about this:

    if(isset($var)) {

    }

    That gets messed up.

    I also added function_exist() in there too to make sure it doesnt mess up with javascript functions.
     
    Kalyse, Apr 19, 2007 IP
  8. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #8
    Nevermind, My bad sorry.

    I misread the regex.

    if (!in_array($function, $allowed_functions) and function_exists($function)) and it works great.

    Thanks.
     
    Kalyse, Apr 19, 2007 IP
  9. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #9

    It doesn't mess up if you add isset to the list of allowed functions.


    And be careful with the function_exists, exit(), and die() are language constructs, and not functions, and therefore it won't trigger there.
     
    nico_swd, Apr 19, 2007 IP
  10. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #10
    O, there is no way of them from being able to create their own function is there?

    function myfunction() {


    }

    Actully now I think about it, who cares... they cant exploit it in anyway.
     
    Kalyse, Apr 19, 2007 IP
  11. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #11
    Thanks for the heads up.
    So to stop people from entering exit and die() I have to do an additional check for them?
     
    Kalyse, Apr 19, 2007 IP
  12. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #12
    EDIT: ^^ Yes, and also this:


    And what about includes/requires? Are you gonna allow them? They work without parenthesis as well, so you'd need an extra check for them.

    EDIT 2:

    eval() in a language construct too, which you should take care of.
     
    nico_swd, Apr 19, 2007 IP
  13. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #13
    No, not allow includes.
    So what must I do with eval?
    Its not a function right?
     
    Kalyse, Apr 19, 2007 IP
  14. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #14
    The difficult thing about banning these, is that they could be words, and part of a text. However, something like this would be the easy way:

    
    if (preg_match('/((require|include)(_once)?|exit|die|eval)/i', $code))
    {
        // Error
    }
    
    PHP:
     
    nico_swd, Apr 19, 2007 IP
  15. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #15
    I dont mine forcing the user to use a theasaurus :) Security is better

    Thanks for your continued help
     
    Kalyse, Apr 19, 2007 IP
  16. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #16
    Actually, this is going to be more complicated.

    What if someone enters:
    
    
    $echo = 'fsockopen';
    
    $fp = $echo($host, $port);
    
    
    PHP:
    ??
     
    nico_swd, Apr 20, 2007 IP
  17. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #17
    Ugh.... I never thought about that. Theres no way of stopping this is there?

    On the plus side, I decided I am going to buy Zend...

    So I dont care that much any more.
     
    Kalyse, Apr 20, 2007 IP
  18. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #18
    Strange no one else likes to join in but anyway.

    Should I just do a :

    preg_match('/\$(.)*\((.)*\)/' bla bla

    I know that very rough and even that could be exploited.. I can think of ways to get around it, but should I do soemthing like that? (Just tidy up the regex)
     
    Kalyse, Apr 20, 2007 IP
  19. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #19
    This could do it, yes.

    
    
    if (preg_match('/\$\w+[\r\n\s\t]*\(/', $code))
    {
         exit('For security reasons, variables cannot be used as function holders');
    }
    
    
    PHP:
     
    nico_swd, Apr 20, 2007 IP
  20. Kalyse

    Kalyse Peon

    Messages:
    1,221
    Likes Received:
    24
    Best Answers:
    0
    Trophy Points:
    0
    #20
    Many thanks again.
     
    Kalyse, Apr 20, 2007 IP