Fixes by NY

[Nijikokun]

Fixing login: Username issues

I usually type my username in as: Username, when installing I typed in: username.

Issue? Uppercase "u".

File

Code:

admin/inc/login_functions.php

Line 60

Original

Code:

if ( $userid == $USR and $password == $PASSWD ) {

Fixed some coding as well to reduce collisions.

Code:

if ( (strtolower($userid) == strtolower($USR)) && ($password == $PASSWD) ) {

Update cookie fix to show real username instead of one you type - just my personal preferences

Line 95

Original

Code:

setcookie($cookie_name, $userid, time() + $cookie_time);

Fixed

Code:

setcookie($cookie_name, $USR, time() + $cookie_time);

[Nijikokun]

Fix: Instead of basename();

warning all files except ajax php files

Files admin/inc/basic.php, admin/inc/login_functions.php, admin/inc/template_functions.php

Lines: usually 10 - 12

Search to find it

Code:

if (basename($_SERVER['PHP_SELF'])

Replace:

Code:

if (basename($_SERVER['PHP_SELF']) == 'filename was here') {
        die('You cannot load this page directly.');
    }

With:

Code:

if (!defined('IN_GS')) {
    die('You cannot load this page directly.');
}

Insert Into admin/*.php except in cron.php do not replace place after, index.php, admin/inc/changedata.php

BEFORE inclusions, require_onces, after base comment code block [top of the file after comment block]

Code:

// Telling the site we can include.
define('IN_GS', true);

[Nijikokun]

Fix: Small/Large security hole in 1.71

Open admin/inc/base.php
Insert at the bottom after in_arrayi(); function

Code:

/*******************************************************
* @function alpha
* @param $string - the string / number / ect
* @param $check - what are we making sure the $string is?
* @about - checks a string / ect against regular expressions
*
*/
function alpha($string, $check = 'alpha')
{
    switch($check)
    {
        case "alpha":
            $regexp = "([a-z0-9])";
        break;
            
        case "alpha-space":
            $regexp = "([a-z0-9+ ])";
        break;
            
        case "alpha-underscore":
            $regexp = "([a-z0-9\_])";
        break;

    case "alpha-slug":
        $regexp = "([a-z0-9\_\-])";
    break;
            
        case "alpha-spacers":
            $regexp = "([a-z0-9-[\]_+ ])";
        break;
            
        case "num-dash":
            $regexp = "([0-9-])";
        break;
            
        case "alpha-extra":
            $regexp = "([a-z0-9+ \\\,\?\`\'\!\.\;\:\[\]\&\%\^\*\$\@\(\)\<\-\_\+\=])";
        break;
            
        case "num":
            $regexp = "([0-9])";
        break;
            
        case "strict":
            $regexp = "([a-z])";
        break;
    }
    
    return (preg_match('/^'.$regexp.'+$/i',$string)) ? true : false;
}
/******************************************************/

Open index.php, not admin/index.php
Find

Code:

// get page id (url slug) that is being passed via .htaccess mod_rewrite
    if (isset($_GET['id'])) {
        $id = strtolower($_GET['id']);
    } else {
        $id = "index";
    }

Replace With

Code:

if (isset($_GET['id'])) {
        if(alpha($_GET['id'], 'alpha-slug'))
        {
            $id = strtolower($_GET['id']);
        }
        else
        {
            $id = "index";
        }
    } else {
        $id = "index";
    }

Save and enjoy a little more security!

[Nijikokun]

I hope these are easy to understand, if not just ask.

I only started fiddling with this cms about two hours ago.

[Zegnåt]

First of all, if you want to make your installation more ecure why would you make your username case-insensitive? You wouldn’t catch me doing that.

I do like the way you handled the checking whether files are included or not. But does it really make the checking that much better or smoother?

Thanks for the security fix, but please tell me why we should add such an enormous checking function when we could just take the regex for “alpha-slug” only? Also, should slugs really only contain those signs? To illustrate this, I should be free to have the name zegnåt as slug right? I think that will need a little revising before actually be implemented into the GetSimple core.

Love your input! Be sure to let us know if you find anything else.

[Nijikokun]

Url disputing: well urls should only be alphanumerical only. since it uses that for slugs I would assume that.

Checking files: yes, its safer for different types of servers as well since the function basename works differently on most.

Username dispute: Its for me personally most sites do it anyway, lastfm, myspace, twitter, ect. It's not a security issue on the username, the seperation part with parenthesis is more secure coding at least.

The huge function its a great function that simplifies having thirty different alpha[name here] check functions into one. It can be used throughout the whole script to secure more things and probably is one of the best functions I've made. I implemented it into codeigniter, cake, symphony, it's universally a great function that should be fundamentally in the top ten of any php programmer.

Examples:

Code:

if(!alpha($username)){ $error = "username must be alphanumeric"; }
if(!alpha($number, 'numeric')){ $error = $thats not a valid number, negative numbers do not apply."; }

Its just a great function. Don't throw away the rest for one thing, they could be used for something else without having to create another function.

[Nijikokun]

URGENT if you used the fix from earlier

admin/inc/changedata.php needs to have this code:

Code:

// Telling the site we can include.
    define('IN_GS', true);

[ccagle8]

NY: I am curious as to why this is a security hole? All it does is takes the $_GET['id'] variable and looks for a file with the same slug. If it doesn't get an ID, it goes to the homepage, and if it gets an ID that doesn't exist it throws up a 404.

To me this doesn't sound like a security hole at all.


Also, in your first post I agree with Zegnat that i like the username check the way it is. As for the cookie function, this function is changed around in version 2.0, and wont be used quite like that anymore.

[ccagle8]

NY: I am not sure why this and this are URGENT. There is nothing wrong with the way we do things now, and if I am missing something, please PM me the details.

Thanks for your insight on our project. Please don't take my posts as hostility, I am only merely tying to understand your issues, and if they have a place in 2.0.

[Nijikokun]

The id thing really isn't a huge security hole however it could be used maliciously when checking for a file. Plus, better safe than sorry anyway.

The login thing isn't such a huge deal its just a personal preference as I stated. Its not a security risk either so it's a safe fix around.

The define however is a WAY better solution rather than checking if the user is accessing the file because some servers don't allow use of those variables or may just treat them differently where as you can always define and check for defined variables in php. Inclusion from remote server will not allow that inclusion where as you can trick the php file into thinking that they are on it.

-- the pm, theres no hack oriented details.

On the urgent post, I just had forgotten a file in that tutorial on the fix and really didn't think about how someone might take that post if they only read that one. Sorry.

[Zegnåt]

Url disputing: well urls should only be alphanumerical only. since it uses that for slugs I would assume that.

What with IDN and the way today’s OS work with Unicode even in file names I think that way of thinking is a little old fashioned. One thing I could find myself agreeing with is to ban certain forbidden signs like slashes and colons and send those directly to the 404.

[Nijikokun]

Url disputing: well urls should only be alphanumerical only. since it uses that for slugs I would assume that.

What with IDN and the way today’s OS work with Unicode even in file names I think that way of thinking is a little old fashioned. One thing I could find myself agreeing with is to ban certain forbidden signs like slashes and colons and send those directly to the 404.

It is old fashioned but php's native support of them is a bit rough and takes a bit of functions to regexp. I personally know the regexps for japanese / some latin fonts and thats about it. I do agree with banning forbidden signs though.