PHP function question

Hi all,

I’ve got a function in one of my php scripts that periodically errors out with


[Wed Jan 21 11:20:21 2009] [error] [client 10.0.10.13] ALERT - canary mismatch on efree() - heap overflow detected (attacker '10.0.10.13', file '/srv/www/htdocs/sales_by_department.php', line 1335), referer: http://linux-aqep/sales_by_department.php

the line 1335 is the line calling the function get_LY_Non_Merch($Location, $FromDate, $ToDate) (see below).

I’ve added a couple of usleep(xx)'s into the code which seems to help as the error isn’t consistant (i.e., it only happens occasionally). Most of the time the script completes.

The reason I use the pear::DB module for one database and odbc_connect() for the other is that originally I had them both connecting with pear::DB but couldn’t figure out how to set the ‘SQL_CUR_USE_IF_NEEDED’ with it so I just switched it to odbc_connect().

Here’s the function:


function get_LY_Non_Merch($Location, $FromDate, $ToDate){
$sql = "
SELECT DISTINCT
	((SUM((SalesHistoryDetail.SaleAmt)) + SUM((SalesHistoryDetail.SaleDisc))) - (SUM((SalesHistoryDetail.RtnAmt)) 
	+ SUM((SalesHistoryDetail.RtnDisc))) - (SUM((SalesHistoryDetail.SaleDisc)) - SUM((SalesHistoryDetail.RtnDisc)))) AS NonMerch
FROM
	SalesHistoryHeader
INNER JOIN
	SalesHistoryDetail
			ON
		SalesHistoryHeader.SHMID = SalesHistoryDetail.SHMID
INNER JOIN
	Location
			ON
		SalesHistoryHeader.LocationID = Location.LocationID
INNER JOIN
	SalesTypes
			ON
		SalesHistoryDetail.TypeID = SalesTypes.TypeID
WHERE
	SalesTypes.Description = 'Non-Merch'
AND
	SalesHistoryHeader.PostDate >= '$FromDate'
AND
	SalesHistoryHeader.PostDate <= '$ToDate'
AND
	Location.Description = '$Location'
";

$dsn = "Winprism";
$user = "readonly";
$pass = "passwd";
$db = odbc_connect($dsn, $user, $pass, SQL_CUR_USE_IF_NEEDED);
$q = odbc_exec($db, $sql);

$db2 = DB::connect("mysql://klucas:passwd@localhost/sales_by_department");
	if (DB::iserror($db2)) {
	die($db2->getMessage());
	}
while (odbc_fetch_into($q, $row)){
$sql3 = "
UPDATE
	sales_by_department.dcc_sales
SET
	dcc_sales.LYNetSales = '$row[0]'
WHERE
	dcc_sales.Department = 'NM'
";
usleep(4);
send_query($sql3, $db2);
} // end while
//disconnect($db2);
//odbc_close($db);
return 0;
usleep(20);
} // end function definition for get_LY_Non_Merch()

Any ideas?

Thanks in advance.
kev.

Sounds like you’ve hit a bug inside the PHP interpreter or some binary extension that was loaded. I wouldn’t be surprised. Are you up to date with all the PHP patches?

A search on “canary efree” turned up this explanation of this internal error detection feature in PHP:

Suspekt… » Blog Archive » Suhosin: canary mismatch on efree() - heap overflow detected

this is a deep bug within not so well used extensions. i had had a quick talk with stefan esser about a year and a half ago and he simply told me he was totally sick of the php team to totally ignore these kind of bugs and simply going on to support these extensions without even telling.

the link ken_yap posted is totally right, something wrote over some part of memory it should’nt. since odbc is something rarely used with PHP i would point on that.

however you use mysql, so i give you the advice to use either mysqli (mysql improved) or better PDO.

Thanks for the info. I did run across that article while I was searching for a solution to this. I’m running suse 10.3 on this machine with php 5.2.6. I see that they’re on 5.2.8 now so I’ll try installing that from the php build repo.

BTW, I would also counsel you to use prepared statements with placeholders instead of expanding variables directly in the query string. You will avoid the danger of SQL injection that way. There are some libraries like ado and ado_lite that make it prepared statements easy to use. For example I do things like this in my code:

$rs = dbquery('SELECT UNIX_TIMESTAMP(modified) FROM outline WHERE year=? AND session=? AND uscode=? AND subcode=?', array($y, $s, $u, $sc));
$modified = $rs->fields[0];

I second ken_yap’s suggestion, not to mention you’ll have little trouble getting it to run on a different sql database if the need arises.

Ahhh, yes, good idea. Thanks.

I updated php yesterday and only ran into the original error once on a part of the script that I hadn’t put usleeps in so unfortunately it doesn’t seem to have been corrected in the latest version but it doesn’t seem to come up as often either.

I wonder why you need ODBC anyway. The plugin is probably the source of the bug, and if it only happens if you don’t sleep, then it’s still there, waiting to strike again some day. There’s no problem connecting to two MySQL databases at the same time. I’m sure you can find a way around the reason you want to ODBC.

Well, that’s a good point. The reason that I’m using ODBC in the first place is that the database that I’m connecting to with ODBC is an MSSQL one and the MSSQL functions don’t seem to be in the php as installed from the repos. I did try compiling it in from source a while back but was unable to get it to compile cleanly.

Another way to interface to MSSQL server is via freetds. Unfortunately this usually requires a rebuild of PHP, but since you are already doing it, it may not bother you.

From Perl, one can easily call the freetds library using the sybase DBD module. I used freetds once to suck data from a MSSQL server.

Interesting because I’m already using freetds as the driver in my /etc/unixODBC/odbc.ini file to establish the ODBC socket to the MSSQL database (which the script then uses the odbc_connect() function to connect to.

Would a call to sybase_connect() in place of odbc_connect() do the trick?

Think I may have at least a work around for this. The result of the query is always a single value–to be sure of this I changed the SELECT DISTINCT to SELECT TOP 1–so the odbc_fetch_into() and indeed the whole while loop is unnecessary and I replaced it with


odbc_fetch_row($q);
$field = odbc_result($q, 1);

Then replace the $row[0] in the UPDATE statement with $field.

I’ve run the script several times without running into the canary mismatch even after taking the usleeps() out. <fingers crossed>

The same logic is used in several other functions in the script but they all return multiple values and haven’t been erroring out.

Please file a bug report at http://bugzilla.novell.com, if you have a short reproducible test case, I will take care of it.

Thanks I’ll do that.

Okay, it’s Bug 473915.

If you need more info let me know but I think I’ve pretty much covered it.

Although this was happening less after the bug above was closed it was still coming up once and a while.

So while working on another script I came across a solution using the prepare and execute methods in the pear DB class.

Unfortunately, this led to an invalid cursor state error message from the DB.

The issue seemed to have something to do with the open database connection being reused when the function was called. The function sending another select statement caused the invalid cursor state.

So…

A separate function to get the–in this case–locations in an array and return that:


// function to get a list of locations and put them in an array then returns that array
function get_locations(){
$locationsArray = array();
require_once('DB.php');
	$dbl = DB::connect("odbc://XXXXXX:XXXXXX@/Winprism");

	// ...and check for errors.
	if (DB::iserror($dbl)) {
		die($dbl->getDebugInfo());
	};

$sqll = $dbl->prepare("
SELECT
	Location.Description
FROM
	Location
");

if (PEAR::isError($sqll)) {
    die($sqll->getDebugInfo());
}
$ql =& $dbl->execute($sqll);
if (PEAR::isError($ql)) {
    die($ql->getDebugInfo());
}




while ($ql->fetchInto($row)) {
	$Location = $row[0];
	$Location = trim($Location);
$locationsArray] = $Location;
} // end while

return $locationsArray;

$dbl->disconnect();
} // end function definition for get_locations()

Then, instead of using a while loop as in the original version I used a foreach loop to iterate through this array. Only one connection to the ODBC database needed.

So the new version of the get_TY_Non_Merch function goes:


function get_TY_Non_Merch($Location, $FromDate, $ToDate){


require_once('DB.php');

	$dbx = DB::connect("odbc://XXXXXX:XXXXXX/Winprism");
	// ...and check for errors.
	if (DB::iserror($dbx)) {
		die($dbx->getDebugInfo());
	}

$sql = $dbx->prepare("
SELECT TOP 1
	((SUM((SalesHistoryDetail.SaleAmt)) + SUM((SalesHistoryDetail.SaleDisc))) - (SUM((SalesHistoryDetail.RtnAmt)) 
	+ SUM((SalesHistoryDetail.RtnDisc))) - (SUM((SalesHistoryDetail.SaleDisc)) - SUM((SalesHistoryDetail.RtnDisc)))) AS NonMerch
FROM
	SalesHistoryHeader
INNER JOIN
	SalesHistoryDetail
			ON
		SalesHistoryHeader.SHMID = SalesHistoryDetail.SHMID
INNER JOIN
	Location
			ON
		SalesHistoryHeader.LocationID = Location.LocationID
INNER JOIN
	SalesTypes
			ON
		SalesHistoryDetail.TypeID = SalesTypes.TypeID
WHERE
	SalesTypes.Description = 'Non-Merch'
AND
	SalesHistoryHeader.PostDate >= '$FromDate'
AND
	SalesHistoryHeader.PostDate <= '$ToDate'
AND
	Location.Description = '$Location'
");

if (PEAR::isError($sql)) {
    die($sql->getDebugInfo());
}
$q =& $dbx->execute($sql);
if (PEAR::isError($q)) {
    die($q->getDebugInfo());
}


//$q = odbc_exec($db, $sql);
require_once('DB.php');
$db2 = DB::connect("mysql://XXXXXXX:XXXXXXXX@localhost/sales_by_department");
	if (DB::iserror($db2)) {
	die($db2->getDebugInfo());
	}


while ($q->fetchInto($row)){
	$field = $row[0];
} // end while

//odbc_fetch_into($q, $row);
$sql2 = "
INSERT INTO sales_by_department.dcc_sales VALUES ( 'NULL', 'NM', '-', '$field', 'NULL', 'NULL', 'NULL', 'NULL', 'NULL', 'NULL', 'NULL', 'NULL', 'NULL' )
";

// this is a custom function that does what it
// says
send_query($sql2, $db2);

disconnect($db2);
disconnect($dbx);
//odbc_close($db);
return 0;

} // end function definition for get_TY_Non_Merch()

and that’s executed in the foreach loop


$locationArray = get_locations();
foreach ($locationArray as $key=>$Location){
refresh_temp_table();
get_department_list();
get_sales_data_TY($Location, $TYDateFrom, $TYDateTo);
get_sales_data_LY($Location, $LYDateFrom, $LYDateTo);
get_departments_with_dcc_sales_TY($Location, $TYDateFrom, $TYDateTo);
get_departments_with_dcc_sales_LY($Location, $LYDateFrom, $LYDateTo);
get_TY_Non_Merch($Location, $TYDateFrom, $TYDateTo);
get_LY_Non_Merch($Location, $LYDateFrom, $LYDateTo);
get_totals();

print "<td>";
print_output($fBorder, $Location, $TYDateFrom, $TYDateTo, $LYDateFrom, $LYDateTo);
print "</td>";