• Registered: 2004-11-22
  • Posts: 8

Topic: Potential Security Flaw in includes/common.php

If a user accesses includes/common.php directly they will see a reference to $pun_root in an include. If register globals is On, this will allow an attacker to access other php scripts with the name config.php. Further more, on some versions of PHP (I believe <= 4.1.0) a %0 can be used to put a NULL in a GET parameter, so any file could be accessed. For example:

pun_root=../../etc/passwd%0

Could lead to etc/passwd being read (of course, you need the correct amount of directory escapes and permissions to read the passwd file)

For a lot of people, this vulnerablity should not cause much threat (none if register globals is off). That is why I decided it would be okay to post it in the public forum (also, could not find appropiate contact information to disclose privately). However, for a small minority it could cause a problem.

Good Luck

2 Reply by anythingwilldo (edited by anythingwilldo 2004-11-22 11:23)

  • Registered: 2004-11-22
  • Posts: 8

Re: Potential Security Flaw in includes/common.php

Edit Reason: Inital though that is now irrelevant

  • From: 127.0.0.1
  • Registered: 2001-11-02
  • Posts: 9,167

Re: Potential Security Flaw in includes/common.php

But this was dealt with in 1.1.5?

@include $pun_root.'config.php';

was replaced with

if (is_dir($pun_root)) @include $pun_root.'config.php';

The result is that $pun_root must point to a valid directory.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Rickard's Website

  • From: 127.0.0.1
  • Registered: 2001-11-02
  • Posts: 9,167

Re: Potential Security Flaw in includes/common.php

anythingwilldo wrote:

(also, could not find appropiate contact information to disclose privately)

My bad. I will add some info to the site on how to report potential security problems when I get home from work.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Rickard's Website

5 Reply by anythingwilldo (edited by anythingwilldo 2004-11-22 11:21)

  • Registered: 2004-11-22
  • Posts: 8

Re: Potential Security Flaw in includes/common.php

Rickard wrote:

But this was dealt with in 1.1.5?

@include $pun_root.'config.php';

was replaced with

if (is_dir($pun_root)) @include $pun_root.'config.php';

The result is that $pun_root must point to a valid directory.

Ah, I thought there was a reason why remote includes did not first come to mind. However, my first post is still relevant.

Say my location is /home/username/ and there is a directory name 'directory'

then:

echo is_dir('./directory');
echo is_dir('../username/directory');

Provided I made no error, is_dir should return true (and the integer 1 will be printed). Basically, other config.php files could be included. Not much of an issue, but something small that could possibly cause a problem when other scripts are running besides PunBB.

  • From: 127.0.0.1
  • Registered: 2001-11-02
  • Posts: 9,167

Re: Potential Security Flaw in includes/common.php

True. It's a far less serious issue, but it does need taking care of. Do you have any suggestions? The problem is that pun_root must still be allowed to contain a proper relative path, so just stripping out '..' won't do.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Rickard's Website

  • Registered: 2004-11-22
  • Posts: 8

Re: Potential Security Flaw in includes/common.php

Well, I suppose checking the existence of pun_root would do:

if (!isset($pun_root)) {
    exit('If $pun_root does not exist, then this script is not being included from a valid file');
}
  • From: 127.0.0.1
  • Registered: 2001-11-02
  • Posts: 9,167

Re: Potential Security Flaw in includes/common.php

Hmm, I'm not following you here. What good would that do? We want to prevent people from supplying common.php with a pun_root via GET.

One possible solution is to check whether $_GET contains an index 'pun_root' and if it does, just exit.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Rickard's Website

  • Registered: 2004-11-22
  • Posts: 8

Re: Potential Security Flaw in includes/common.php

sorry, wasn't thinking:

if (!isset($pun_root) || isset($_REQUEST['pun_root']))

Gotta wake up today! tongue

  • From: 127.0.0.1
  • Registered: 2001-11-02
  • Posts: 9,167

Re: Potential Security Flaw in includes/common.php

Hehe, no worries. I'll add something along those lines to 1.2. Thanks.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Rickard's Website

  • From: 127.0.0.1
  • Registered: 2001-11-02
  • Posts: 9,167

Re: Potential Security Flaw in includes/common.php

I'll solve it by changing $pun_root to a constant instead. That way, I won't have to "globalize" it in every function.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Rickard's Website

  • From: Lima - Perú
  • Registered: 2004-09-23
  • Posts: 104

Re: Potential Security Flaw in includes/common.php

hey rickard you can post a solution for this bug in forums punbb 1.1.5 plz

El Mejor Lugar de la Red - Corporación Azakur4

Azakur4's Website

  • Connorhd
  • Connorhd
  • Former PunBB Developer
  • Offline
  • From: Leeds, UK
  • Registered: 2004-06-13
  • Posts: 4,195

Re: Potential Security Flaw in includes/common.php

change $pun_root to a constant in every file... its not worth it just wait for the 1.2 release wink

FluxBB.
Connorhd.co.uk.

Connorhd's Website