1

(3 replies, posted in Feature requests)

I think adding php bbcode tags (simalar to those found in vBulletin) is a good idea. For example:

[PHP]
<?php
echo "This is example code, no real functionility";
?>
[/PHP]

This is easy to implement because PHP comes with all needed functions (and pun bb is written in PHP). It just makes PHP code much easier to read and better looking. Obviously not a absolute critical feature but a useful one all the same.

I failed to realise the post was 10 pages long, probably more appropiate I guess to post this issue at www.punres.org I suppose.

The is a potentially dangerous line of code in many of those files:

require $pun_root.'lang/'.$language.'/'.$language.'_pms.php';

Without validation of where the variables originate from (ie. common.php or the GET interface) problems could arise. If any of these are true, then this is dangerous.


* Obviously, register globals must be on.
* PHP 4.1.0 or less - A null can be put on strings meaning any file can be included, not just php files.
* If the version of PHP is 4.3.0 or greater, and allow_url_fopen is on, remote files can be included


But, the problem is that even if that particular vulneribility isn't exploited successfully there is plenty more. Many of the scripts, if accessed directly, make no checks of where the variables originated from. This can lead to XSS, SQL Injection, session hijacking, etc, etc. Best solution would be to check if config.php is loaded:

if (!defined('PUN')) {
    exit('This file is not meant to be loaded directly');
}

And obviously if the page is meant to be accessed directly devise a more indepth check to see where variables came from.

I don't know if this truly an issue for you guys or not. If you require register_globals to be off (I don't believe you do) then it is secure or if you expect people to devise protection themselves on individual files (which I don't believe you do).

sorry, wasn't thinking:

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

Gotta wake up today! tongue

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');
}
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.

Edit Reason: Inital though that is now irrelevant

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