what does this mean

[msnhockey]

everytime I search for thehockeystop online I get the following on alot of searches

HockeySTATS Online is a PHP-based application for tracking hockey statistics.

The application is prone to multiple SQL-injection vulnerabilities because it fails to sufficiently sanitize user-supplied data to the 'id' and 'divid' parameters of the 'index.php' script before using it in an SQL query.

A successful exploit may allow an attacker to compromise the application, access or modify data, or exploit latent vulnerabilities in the underlying database.

HockeySTATS Online Basic and Advanced 2.0 are vulnerable; other versions may also be affected.

this is a stats script i bought and I want to know how bad it is and how I can change it so I am not so vulnerable

thanks

[laserlight]

How bad it is could be summarised by this comic: Exploits of a Mom.

Fixing the problem depends on the database extension used. For example, we might use a prepared statement instead of building SQL statements by concatenating the values of incoming variables, or we might use the proper escaping function (typically some function provided by the database extension for strings and a type cast for numeric values) when concatenating such values.

[msnhockey]

here is the index.php original file that it came with.. not sure if you could tell me where this exploit is or what to change

<?php
/**********************************************
*       Based on CCLeague PRO Soccer by Orlando L. Castillo
*
*       GPL software
*
*       Original Author:  Orlando L. Castillo
*
*    -----------------------------------------------
*
*       Revised by Darin Jones for Ice Hockey and Lacrosse
*	Web: http://www.thehockeystop.com/site
*
*	10-08-03 Fixed bugs, redesigned database, added
*       many new features, visit changelog at thehockeystop.com
*       for the detailed list
*
*       Requires: PHP 4+ and MySQL
*
*       Support forums and FAQ's are located at
*       http://www.thehockeystop.com/site
*
**********************************************/

 include("config.php");

//FIND OUT THE LANGUAGE
if($_POST["setlanguage"]) {
        setcookie("language");
        setcookie("language",$_POST["setlanguage"],time()+1209600,"","","");
	$lang = "./lang/".$_POST["setlanguage"].".ini";
	$llang = $_POST["setlanguage"];
} elseif($_COOKIE["language"]) {
	$lang = "./lang/".$_COOKIE["language"].".ini";
	$llang = $_COOKIE["language"];
} else {
	$l_array = explode("-",$lang_array[0]);
	$llang = $l_array[0];
	$lang = "./lang/".$l_array[0].".ini";
}


//LOAD THE HEADER
$header = $sport_dir."html/header.html";
$fd = fopen ($header, "r");
$output = fread ($fd, filesize ($header));
fclose ($fd);

switch($opt) {

    //VIEW TEAM INFORMATION
    case "viewteam":

    include("team.php");
    $output .= view_team($id,$sid);

    break;

    //VIEW TEAM INFORMATION
	case "viewplayer":

	include("player.php");
	$output .= view_player($pid,$divid,$sid,$tid);

    break;

    //VIEW HTML OR ANNOUNCEMENT
    case "viewpage":

    include("html.php");
    $output .= html($id,$type);

    break;

    //SHOW ALL ANNOUNCEMENTS
    case "ahomecomplete":

    include("announcement.php");
    $output .= ahomecomplete();

    break;

    //VIEW SCHEDULE AND SCORES
    case "schedule":

include("standings.php");
//if(!$thisdiv) $thisdiv = "none";
//if(!$season) $season = $open_season;
$output .= listteams($divid,$season);

break;

      //DIVISION STATS
        case "divlead":

	include("divlead.php");
	//if(!$thisdiv) $thisdiv = "none";
	//if(!$season) $season = $open_season;
	$output .= divstats($divid,$sid);

break;

//LOGIN FORM
case "login":

include("login.php");
$output .= login();

break;

    //LOGIN CHECK
    case "admincheck":

    include("logincheck.php");
    $output .= logincheck($fusername,$fpassword);

    break;

    //CREATE AN ADMINISTRATOR
    case "createadmin":

    include("createadmin.php");
    $output .= createadmin();

    break;

    //SQL ADMINISTRATOR FUNCTION
    case "createadmin2":

    include("createadmin.php");
    $output .= createadmin2($adminpassword);

    break;

//HOME
default:
$output .= admincheck($hostname,$user,$password,$db,$lang);

}

//REPLACE CUSTOM HTML, ETC.
function replace_tags($content) {
	$content = eregi_replace("\n","<br />",$content);
	$content = eregi_replace("<bold>","<b>",$content);
	$content = eregi_replace("</bold>","</b>",$content);
	$content = eregi_replace("<italic>","<i>",$content);
	$content = eregi_replace("</italic>","</i>",$content);
	$content = eregi_replace("<link","<a",$content);
	$content = eregi_replace("</link>","</a>",$content);

return $content;
}

//ADMIN CHECK
function admincheck($hostname,$user,$password,$db,$lang) {

    $open = mysql_connect($hostname,$user,$password);
    mysql_select_db("$db",$open);
    $result = mysql_query("SELECT * FROM teams WHERE username = 'admin'");
    $i = 0;
$total_rows = mysql_numrows($result);
if($total_rows != 1) {
        $output .= "
		<tr><td bgcolor=\"#ffffff\">
		<p align=center class=\"header\"><!--&#37;cc_welcometo%--> Your Hockey League.<br>
		<!--%cc_security%--> <a href=\"./?opt=createadmin\">
                <!--%cc_gotherenow%--></a></p>
		</td></tr>";
	$check = "false";
	$sql = "INSERT INTO leagueinfo VALUES ('NULL','Your Hockey League','',";
	$sql .= "'','','','',";
	$sql .= "'1','10','1','','')";
	$done = mysql_query($sql);
} else {
	include("./home.php");
	$output = home();

}

return $output;
}



$footer = $sport_dir."html/footer.html";
$fd = fopen ($footer, "r");
$output .= fread ($fd, filesize ($footer));
fclose ($fd);


$langselect = "
	<a href=\"setlang.php?lang=".$llang."\" onClick=\"NewWindow(this.href,'name','300','150','yes');return false;\">
	<!--%cc_language%--></a>";

$output = eregi_replace("<!--%LANGUAGESELECT%-->",$langselect,$output);

include("./lang.php");
$output = lang($lang, $output);

echo $output;

?>

[laserlight]

There are no SQL injection vulnerabilities apparent in that code because all the SQL statements executed do not involve incoming variables, or any variables at all, for that matter.

However, it is clear that the MySQL extension is used, so [man]mysql_real_escape_string/man should be used to escape string values before using them in an SQL statement.

[msnhockey]

i should add that this script require register globals to be on.. not sure if that makes a difference

[laserlight]

msnhockey wrote:

i should add that this script require register globals to be on.. not sure if that makes a difference

Yes, that makes things worse, but as I said, in that particular page there is no SQL injection vulnerability, though there may be some in the included files.

You should be looking for things like this:

$result = mysql_query("SELECT * FROM teams WHERE username = '$user' AND id = $id");

Check if $user and $id are correctly sanitised. If they are not, sanitise them, e.g.,

$query = sprintf("SELECT * FROM teams WHERE username = '%s' AND id = %d",
    mysql_real_escape_string($user), $id);
$result = mysql_query($query);

[msnhockey]

ok.. thanks..

I have an idea where to find this stuff in the script.
especially in the config file where it wants the username and password etc...

[msnhockey]

i want to re-do the script as the coding is 6-8 years old, but just don't know where to start to get the register_globals gone and have it well scripted

[laserlight]

Well, before you begin making drastic changes to the script, place it under version control so that you can always go back to earlier versions of the script if you make a mistake, especially one that is hard to undo. Then, work step by step, e.g., try and fix all the SQL injection vulnerabilities first, and then disable register_globals and increase error_reporting to E_ALL (it presumably does not include notices at this time) and use [man]isset[/man] and [man]empty[/man] where potential problems are detected.

[msnhockey]

ok thanks

i tried this

$query = sprintf("SELECT * FROM teams WHERE username = '%s' AND id = %d",
mysql_real_escape_string($user), $id);
$result = mysql_query($query);

and where the %s is i had to change to $u for it to work...

[laserlight]

msnhockey wrote:

and where the %s is i had to change to $u for it to work...

That's wrong. Read the PHP manual on [man]sprintf/man and mysql_real_escape_string().

[msnhockey]

this is where it wants the $u

//SECURITY CHECK - SEE IF USERNAME EXISTS
if($u) {

// session_start( );
// $sessionck = session_id( );

// include("config.php");

$open = mysql_connect($hostname,$user,$password);
mysql_select_db("$db",$open);

$query = sprintf("SELECT * FROM teams WHERE username = '%s'",
mysql_real_escape_string($user));
$teams = mysql_query($query);

[laserlight]

Ah, then what you should do is replace $user with $u. Actually, considering that $user is more descriptive than $u, it would be better to replace $u with $user, but that means looking through say, the forms and links elsewhere.

[msnhockey]

i read the sprintf() and I now see exactly now why you need teh %s or whatever

i used this and it still works

$query = sprintf("SELECT * FROM teams WHERE username = '%s'",
mysql_real_escape_string($u));
$teams = mysql_query($query);