»
« home   paste   Anonymous | Login | Signup for a new account 07-28-2017 16:44 CEST
 
* X »
«
GeSHi - Generic Syntax Highlighter Syntax Coloriser for PHP
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0000015 [GeSHi] core trivial have not tried 10-26-05 07:27 02-18-06 11:52
Reporter BenBE View Status public  
Assigned To nigel
Priority low Resolution fixed  
Status closed   Product Version 1.1.0alpha1
Summary 0000015: Check for valid, unlinked paths
Description Because of a bug in 1.0.7.X you now test include files to be inside the GeSHi-Root. That's all nice, but I'd suggest that you include a second check that those files also represent regular files AND there's no linking\mounting inside the GeSHi directory inside the given path. Although this is a bit slower it raises security a bit further because I doubt there's some admin rerouting the GeSHi subdirectories on the server.
Additional Information
Attached Files

- Relationships

- Notes
(0000034)
nigel
10-26-05 13:32

Do you mean this for 1.1.X or 1.0.X? (If 1.1.X then product version for this bug should be 1.1.1alpha1).

If 1.1.X, I guess you mean to change the geshi_can_include function, which is currently:

/**
 * Checks whether a file name is able to be read by GeSHi
 *
 * The file must be within the GESHI_ROOT directory
 *
 * @param string The absolute pathname of the file to check
 * @return boolean Whether the file is readable by GeSHi
 */
function geshi_can_include ($file_name)
{
    return (GESHI_ROOT == substr($file_name, 0, strlen(GESHI_ROOT)) &&
        is_file($file_name) && is_readable($file_name));
}

The question is, how do I detect that I have followed a link?

I could always grep the source to see where this function is called and work out exactly what is allowed and check for that I suppose.
 
(0000035)
BenBE
10-27-05 08:03
edited on: 10-27-05 08:26

Yes, exactly, I mean this proc. I intented the extension for 1.1.X or might be later as it has really low value for server attacks... If it's in 1.1.2 it's still early enough...

I just consulted the PHP online documentation and found a nice example for testing directories to be symbolic links: http://www.php.net/manual/de/function.is-link.php [^] You just have to got through the source there while(($path inside GESHI_ROOT)&&(!is_link(path)) $path = pathof($path); where path is initialized to pathof($filetoinclude).
Add a check is_file in advance or after that loop to ensure the requested file is a regular one.

I guess that should do for that fix.

 
(0000036)
nigel
10-27-05 15:12

Right, I'll do that. Thanks for the detailed description.
 
(0000282)
nigel
01-29-06 15:03

I'm wondering about this bug... is it really GeSHi's fault if the system admin allowed another script to create a symlink to, say, /etc?

e.g. let's suppose that GeSHi's use of including was going to be used as a security hole. The hacker would have to somehow get their script AND GeSHi installed. And if the sysadmin is installing something that creates symlinks in another program's (e.g GeSHi) space, then they should really be shot :)

And also, in some cases people may have set up a symlink for a completely valid reason. I have them set up myself.

So I don't feel that there needs to be anything done about this. Do you have another opinion?
 
(0000284)
BenBE
01-30-06 00:49

I see this issue two-sided:

On the on hand you will probably take advantage of the Unix file system using symbolic\hardlinks thus this protection would be a harm for you, but

on the other hand should each piece of software that utilizes the file system providing access to include (arbitary) files have a protection against abuse.

I ACK on shooting all admins having their software create symlinks in an uncontrolled manner, but each such link is a probable target for attacks. I already "hacked" a remote server using security holes of the production website. One of those bugs was an "PHP include file issue" allowing me to "include" directory listings into the site output or executing arbitary files (even images files if you just tested out the file structure well enough). If that server had had any software I knew of security holes I could use by simply specifying the right params I had used them. Uploading any special scripts was absolutely unneccessary because the page alowed me to do all the stuff I wanted.

Thus I'm for implementing such an restriction with the possibility to deactivate it quite easily if the symlinks are required. Such that you could introduce a const "ALLOW_SYMLINK_PATHS" defaulted to false. Everyone that really requires them could allow them and disable that additional protection.

Symlinks inside the webspace htdocs directory are quite uncommon AND workaround the security system introduced by the webserver, thus they should be avoided anyway.

2ct
 
(0000291)
nigel
01-30-06 09:40

Well as it is GeSHi won't include any files that are not inside its directory. Because all paths are checked to make sure they start with GESHI_ROOT.

To enforce that, all that needs to be done is to do a str_replace('..', '', $file_name).

That way all files _must_ be inside GESHI_ROOT, unless the user themselves created a symlink inside the GeSHi directory to somewhere.

Now, if the user decides to do that, or if they are hacked, it doesn't matter: it's not our problem. If they decided that they wanted the GeSHi files on another partition because they are running out of room, I say they have every right to symlink the geshi directory away. And if they install software that creates such a symlink, well that's their tough luck, but it's not our problem.

And even if the symlink was created in the GESHI_ROOT path somewhere, that's not our problem either. That's an issue with the security of the system that the sysadmin should have picked up. Again, maybe they _wanted_ it that way.

As for symlinks not often being used: this is true, but at the same time I've used it myself on several occasions. For example, to point several sites at the same set of images/css, or to get access to a library.

If you can explain a case whereby geshi_can_include + the str_replace('..', '') returns true for a file yet that file isn't one that GeSHi meant to include, _and_ it's GeSHi's fault, then maybe this is worth doing. Otherwise it just wastes CPU cycles. I'm unaware of any other PHP software that makes checks like this.
 
(0000293)
BenBE
02-01-06 00:10
edited on: 02-01-06 00:37

k, here's the promised source snippet:

/**
 * Checks whether a file name is able to be read by GeSHi
 *
 * The file must be within the GESHI_ROOT directory
 *
 * @param string The absolute pathname of the file to check
 * @return boolean Whether the file is readable by GeSHi
 * @todo [blocking 1.1.5] Check that path does not contain links etc (bug 15)
 */
function geshi_can_include ($file_name)
{
    //Do the root path stuff
    $can_include = GESHI_ROOT == substr($file_name, 0, strlen(GESHI_ROOT));

    if(!$can_include) {
        return false;
    }

    //Check if the given file exists
    $can_include = file_exists($file_name);

    if(!$can_include) {
        return false;
    }

    //Check if it's a regular file
    $can_include = is_file($file_name) && is_readable($file_name);

    if(!$can_include) {
        return false;
    }

    //Check if the user tries to use .. inside the path
    $can_include = false == strpos($file_name, "..");

    if(!$can_include) {
        return false;
    }

    //Check if we need to check for symlinks and if all required functions are available
    //The availability check is due to compatibility to M$ Windows as PHP doesn't implement some symlink functions there.
    if(!GESHI_ALLOW_SYMLINK_PATHS && function_exists('is_link')) {
        do {
            //Check for filename being a file OR a directory
            $file_type = filetype($file_name);
            $can_include &= (("file" == $file_type || "dir" == $file_type) && !is_link($file_name));

            //Change to the parent's directory for next test
            $file_name = dirname($file_name);
        } while (GESHI_ROOT == substr($file_name, 0, strlen(GESHI_ROOT) && $can_include)
    }

    return $can_include;
}


Requires only to introduce a constant like

//Disable Symlinks inside the GeSHi directory
define('GESHI_ALLOW_SYMLINK_PATHS', false);

in class.geshi.php.


As we had no chance for testing a directory with symlinks we cannot tell for sure if it really detects the symlinks, but according to the PHP docs it should. Plz check with your server (if you have symlinks in there) ;-)

 
(0000295)
nigel
02-01-06 09:42

It doesn't look too bad, although it can be shortened. I will test it out.
 
(0000314)
nigel
02-12-06 14:36

Added (and shortened). Works well. I may not do the flag for checking symlinks by a constant in the future, but for now it is. If you want to turn this off you just define GESHI_ALLOW_SYMLINK_PATHS to be false before including class.geshi.php.
 
(0000339)
nigel
02-18-06 11:52

Issue closed.
 

- Issue History
Date Modified Username Field Change
10-26-05 07:27 BenBE New Issue
10-26-05 07:27 BenBE Status new => assigned
10-26-05 07:27 BenBE Assigned To  => nigel
10-26-05 13:32 nigel Note Added: 0000034
10-27-05 08:03 BenBE Note Added: 0000035
10-27-05 08:26 BenBE Note Edited: 0000035
10-27-05 15:12 nigel Note Added: 0000036
01-29-06 15:03 nigel Note Added: 0000282
01-30-06 00:49 BenBE Note Added: 0000284
01-30-06 09:40 nigel Note Added: 0000291
02-01-06 00:10 BenBE Note Added: 0000293
02-01-06 00:37 BenBE Note Edited: 0000293
02-01-06 09:42 nigel Note Added: 0000295
02-12-06 14:36 nigel Note Added: 0000314
02-12-06 14:36 nigel Status assigned => resolved
02-12-06 14:36 nigel Resolution open => fixed
02-12-06 14:36 nigel Fixed in Version  => 1.1.1alpha4
02-18-06 11:52 nigel Status resolved => closed
02-18-06 11:52 nigel Note Added: 0000339

  


Mantis 1.0.0rc2[^]
Copyright © 2000 - 2005 Mantis Group
50 total queries executed.
35 unique queries executed.
Powered by Mantis Bugtracker