I've found a pretty good article
http://education.nyphp.org/phundamentals/PH_storingretrieving.phpThey recommend disabling magic_quotes_gpc then fixing all the magic_quotes if they exist (which we do in init.inc.php)
Secondly they recommend:
Use mysql_escape_string if you are using a version of PHP prior to ver. 4.3.
Use mysql_real_escape_string if you are using PHP ver. 4.3 or better.
When writing to the database.
Thirdly keep the data in as close to raw format as you can while your processing it.
Then use an approrpriate function when you go to display the data; ie: you would use htmlentities if displaying to html, but you could output raw to a csv, just by adding whatever is needed for csv.
If you use htmlentities too soon, then you have to decode when you go to use that data for something OTHER than displaying in html. (it looks like we do a subset of htmlentities in the santization process in init.inc.php)
Also our current sanitation process doesn't remove single quotes from the stream but we do stripslashes, leaving us open to sql injection attacks unless we make sure we are doing some kind of sanitation before writing to the database. (from what I've seen the code in coppermine seems to do this pretty well)
FYI: php.net docs say both of those mysql_escape functions are for 4.3 or greater
also in my tests mysql_escape_string is none-to-bright as it will escape things that are already escaped causing duplicate slashes... and your back where you started.
If we do ANYTHING in init.inc.php we will break a lot of code.
ie: if we double up the single quotes, later in the code when we do an addslash, we will get \'\' written to the database, which isn't what we want.
The moral of the story is education and consistency. Anytime we use get post or cookie variables either directly or indirectly we have to assume the single quotes are unescaped, and cast the var as an int (int)$_GET['cat'] or use addslashes($_GET['title']) for text fields.
I saw a semi-standard going through coppermines code. If we could formulate it as a rule I think that would help. (if it already wasn't)
Never use the gpc globals directly in an sql statement.
take this example from db_input.php
$aid = (int)$_POST['aid'];
$title = addslashes(trim($_POST['title']));
$category = (int)$_POST['category'];
$description = addslashes(trim($_POST['description']));
$keyword = addslashes(trim($_POST['keyword']));
$thumb = (int)$_POST['thumb'];
$visibility = (int)$_POST['visibility'];
$uploads = $_POST['uploads'] == 'YES' ? 'YES' : 'NO';
$comments = $_POST['comments'] == 'YES' ? 'YES' : 'NO';
$votes = $_POST['votes'] == 'YES' ? 'YES' : 'NO';
$password = $_POST['alb_password'];
$password_hint = addslashes(trim($_POST['alb_password_hint']));
$visibility = !empty($password) ? FIRST_USER_CAT + USER_ID : $visibility;
if (!$title) cpg_die(ERROR, $lang_db_input_php['alb_need_title'], __FILE__, __LINE__);
if (GALLERY_ADMIN_MODE) {
$query = "UPDATE {$CONFIG['TABLE_ALBUMS']} SET title='$title', description='$description', category='$category', thumb='$thumb', uploads='$uploads', comments='$comments', votes='$votes', visibility='$visibility', alb_password='$password', alb_password_hint='$password_hint', keyword='$keyword' WHERE aid='$aid' LIMIT 1";
} else {
$category = FIRST_USER_CAT + USER_ID;
$query = "UPDATE {$CONFIG['TABLE_ALBUMS']} SET title='$title', description='$description', thumb='$thumb', comments='$comments', votes='$votes', visibility='$visibility', alb_password='$password', alb_password_hint='$password_hint',keyword='$keyword' WHERE aid='$aid' AND category='$category' LIMIT 1";
}
Make the variables pretty (sanitized) then and only then use them in an sql query.