Efficient Way to Log

[KGodel]

Hey all.

So, I recently added a simple system to my gaming clan forums where staff could grant points to users who attend events. There are several different input methods, which shouldn't be an issue since they write to the database the same way. I am looking to add logging to the system, but I know that this can lead to a very large database. What is the best way to store a log? Database, file editing, etc. I am sure I could run a task to prune them after a certain length of time to keep size down, but yea, best way to log actions?

[Dave]

You could use the adminlog table to log the actions and prune those logged entries every now and then. Take a look at the adminlog table and you'll see that it's very easy to use it.

[KGodel]

Okay, a security question. So I need to log a list of userIDs when what is passed through the script is a list of usernames the staff member types in, and those users are given points. Since the array is passed through cleaning via GPC, is it safe to use that list to check values against to get a list of userIDs, or is there a better way that will leave me less vulnerable to injection?

[Dave]

Depends on what type of array you define in the GPC function. Regardless, I would always use the escape_string function over user input values and check if numbers are really numbers with a function such as ctype_digit.

[kh99]

Yeah, I agree with what Dave said. If you clean it with TYPE_INT or something so that you know it's a number, then you can safely use it in an sql string. But if it's a string you should always use db->escape_string() to include it in sql (no matter what TYPE_ you used to clean it), not only to avoid a security problem but also because if the string contains any special characters (like a quote) then it would cause an error if you haven't escaped it.

[KGodel]

All good tips, thanks guys. Is there any function within VB I can use that can get a UserID based on a username without needing the query the database at all? That would be the ideal, but if not I'll just have to double clean the array. Here is what I have:

$vbulletin->input->clean_array_gpc('p', array(
'points' => TYPE_UNIT,
'users' => TYPE_STR
));

$ausers = $db->escape_string($vbulletin->GPC['users']);
$addusers = explode(";",$ausers);

So addusers is an array. Obviously I'll have to turn it into a list before I can use it in the manner I want (basically query the database to get the ID of anyone whose username is in this list of names). If it's safe after these cleaning methods then I'll be a bit more confident.

[kh99]

I don't know offhand if there's a function to get the id form the username. I don't remember seeing one. But if there is I'm sure it would just do a query like "select userid from user where username = 'something' ". If you so something like
"SELECT userid FROM ".TABLE_PREFIX."user WHERE username='".$vbulletin->db->escape_string(trim($username))."'"

Then I think you'll be fine without any additional cleaning.

[KGodel]

Would it take more time to get each userID individually like that or check for all userIDs for any username in the list at once?

[kh99]

Yes, you could get them all in one query. That is obviously better for efficiency, although if it's something that's only going to happen occasionally with a few names in the list it probably doesn't matter a whole lot.

Edit: so I guess I'm saying you're right, if I were doing it I probably would go with one query, but I think doing it with separate queries isn't bad.

[nerbert]

I don't remember where I got this stuff but here's some vBulletin code to SELECT based on username


$vbulletin->input->clean_array_gpc('r', array(
'username' => TYPE_STR,
'password' => TYPE_STR
));

$username = strip_blank_ascii($vbulletin->GPC['username'], ' ');
// See VBM-635: &#xxx; should be converted to windows-1252 extended char. This may not happen if a browser submits the form. But from API or user manually input, it does.
// See also vB_DataManager_User::verify_username()
$username = preg_replace(
'/&#([0-9]+);/ie',
"convert_unicode_char_to_charset('\\1', vB_Template_Runtime::fetchStyleVar('charset'))",
$username
);

$user = $vbulletin->db->query_first("
SELECT *
FROM " . TABLE_PREFIX . "user
WHERE username = '" . $vbulletin->db->escape_string(htmlspecialchars_uni($username)) . "'
");

[KGodel]

Alright. Here is my final code. It seems to be working, but I want to make sure I didn't leave any space for an injection.

// Get the UserID of the Staff member accessing the page
$staffid = $vbulletin->userinfo['userid'];

print_cp_header();
echo "<div class='pagetitle'>In Game Activity System</div>";

//Possible Point Values
$pointreasons = array("Attended a Gamenight (+10)", "Attended a Gamenight (+15)", "Attended a Gamenight (+20)", "Attended a Contest/Torunament (+30)");

// Get Divisions //
$sql = "SELECT gamename, ingamename, profilefield FROM " . TABLE_PREFIX . "gamelist ORDER BY gamename ASC";
$result = $db->query_read_slave($sql);
$divisions = array();
while ( $a = mysql_fetch_array($result) ) {
$divisions[] = $a;
$divnames[] = $a[0];
}

/////////////////////// front page
if ( empty($_POST['do']) ) {

////////////////////// add points via division
print_form_header($this_script, 'div');
print_table_header('Add Points via Division');
print_label_row('Notice:', 'Select the division you are entering activity for.');
print_select_row('Divisions', 'division', $divnames);
print_submit_row('Select Division');

////////////////////// add points text area
print_form_header($this_script, 'add');
print_table_header('Add Points');
print_label_row('Notice:', 'Select the point value based on the event.');
print_select_row('Points', 'points', $pointreasons);
print_label_row('Users:', 'Input the usernames of all the users you wish to give the above points to. You must separate names using a semicolon (;).');
print_textarea_row('Users', 'users','',12,100,false,false);
print_submit_row('Add Points');

////////////////////// edit points
print_form_header($this_script, 'edit');
print_table_header('Edit Points');
print_label_row('Username:', 'Enter the username of who you want to edit the points value of.');
print_input_row('Username', 'username');
print_submit_row('Edit Points');
}

/////////////////////// div selected
if ( $_POST['do'] == 'div' ) {

if ( !isset($_POST['division']) ) { rpm_print_stop_back('Please select a division.'); }

$vbulletin->input->clean_array_gpc('p', array(
'division' => TYPE_UNIT
));

// Division Name //
$n = $vbulletin->GPC['division'];
$divname = $divisions[$n][0];
$divign = $divisions[$n][1];
$divfield = $divisions[$n][2];
$+++++eck = 1<<$n;

// Get Main Members
$mainusers = $db->query_read_slave("SELECT user.userid, user.username, userfield. " . $divfield ." FROM " . TABLE_PREFIX . "user AS user LEFT JOIN " . TABLE_PREFIX . "userfield as userfield ON (userfield.userid = user.userid) WHERE (userfield.field5='" . $divname . "') AND (user.username != '') AND (user.usergroupid NOT IN (1,3,4,7,8,20,21,23,45)) ORDER BY username");
if (!$mainusers){ die('Invalid query: ' . mysql_error()); }
// Set the Users Array
$maingameusers = array();
while ($user = mysql_fetch_array($mainusers)) {
if ($user[2] == null) { $ign = "N/A"; } else { $ign = $user[2]; }
$maingameusers[$user[0]] = $user[1] . " - " . $divign . ": " . $ign;
}

// Get Other Game Members
$otherusers = $db->query_read_slave("SELECT user.userid, user.username, userfield. " . $divfield . " FROM " . TABLE_PREFIX . "user AS user LEFT JOIN " . TABLE_PREFIX . "userfield as userfield ON (userfield.userid = user.userid) WHERE (userfield.field5 != '" . $divname . "') AND (userfield.field6 & " . $+++++eck . ") AND (user.username != '') AND (user.usergroupid NOT IN (1,3,4,7,8,20,21,23,45)) ORDER BY username");
if (!$otherusers){ die('Invalid query: ' . mysql_error()); }
// Set the Users Aray
$othergameusers = array();
while ($user = mysql_fetch_array($otherusers)) {
if ($user[2] == null) { $ign = "N/A"; } else { $ign = $user[2]; }
$othergameusers[$user[0]] = $user[1] . " - " . $divign . ": " . $ign;
}

////////////////////// select users to get points
print_form_header($this_script, 'divadd');
print_table_header('Add Points to ' . $divname . ' Members');
print_label_row('Event Type:', 'Select the event type (and point value) to give.');
print_select_row('Points', 'points', $pointreasons);
print_label_row('Main Users:', 'Select the Main-Game users who attended the event.');
print_select_row('Main Users', 'mainusers[]', $maingameusers,null,false,10,true);
print_label_row('Other Users:', 'Select the Other-Game users who attended the event.');
print_select_row('Other Users', 'otherusers[]', $othergameusers,null,false,10,true);
print_label_row('Note:', 'Hold the control key while clicking to select multiple names.');
print_submit_row('Add Points to Selected Users','Reset');

}

/////////////////////// add
if ( $_POST['do'] == 'add' ) {

if ( !isset($_POST['points']) OR empty($_POST['users']) ) { rpm_print_stop_back('Please be sure every required field is filled out before submitting.'); }

$vbulletin->input->clean_array_gpc('p', array(
'points' => TYPE_UNIT,
'users' => TYPE_STR
));

$apoints = $vbulletin->GPC['points'];
if ($apoints == 0) { $addpoints = 10; }
if ($apoints == 1) { $addpoints = 15; }
if ($apoints == 2) { $addpoints = 20; }
if ($apoints == 3) { $addpoints = 30; }
$ausers = $db->escape_string($vbulletin->GPC['users']);
$addusers = explode(";",$ausers);
$allusers = "";
//$adate = date("Y-m-d");

foreach ($addusers as &$u) {
$user = trim($u);
$sql = "UPDATE " . TABLE_PREFIX . "user SET iga_points = iga_points + $addpoints, iga_lastpointsdate = NOW() WHERE username='$user'";
$db->query_write($sql);
$allusers .= "'$user',";
}

// Get the userIDs
$allusers = rtrim($allusers,",");
$alluserids = $db->query_read_slave("SELECT userid FROM " . TABLE_PREFIX . "user WHERE username IN ($allusers)");
if (!$alluserids){ die('Invalid query: ' . mysql_error()); }

while ($u = mysql_fetch_array($alluserids)) {
$userids[] = $u[0];
}

$allids = serialize($userids);

// Add the Log
$sql = "INSERT INTO " . TABLE_PREFIX . "igpointslog (date,staffid,type,points,userids) VALUES (NOW(),'$staffid','add',$addpoints,'$allids')";
$db->query_write($sql);

define('CP_REDIRECT', 'iga_points.php');
print_stop_message('iga_points_added');
}

/////////////////////// add via division
if ( $_POST['do'] == 'divadd' ) {

if ( !isset($_POST['points']) OR empty($_POST['mainusers']) ) { rpm_print_stop_back('Please be sure every required field is filled out before submitting.'); }

$vbulletin->input->clean_array_gpc('p', array(
'points' => TYPE_UNIT,
'mainusers' => TYPE_ARRAY,
'otherusers' => TYPE_ARRAY
));

$apoints = $vbulletin->GPC['points'];
if ($apoints == 0) { $addpoints = 10; }
if ($apoints == 1) { $addpoints = 15; }
if ($apoints == 2) { $addpoints = 20; }
if ($apoints == 3) { $addpoints = 30; }
$addmain = $addother = array();
$addmain = $vbulletin->GPC['mainusers'];
$addother = $vbulletin->GPC['otherusers'];
$allusers = serialize(array_merge($addmain,$addother));
//$adate = date("Y-m-d");

// Main Gamers
foreach ($addmain as &$u) {
$sql = "UPDATE " . TABLE_PREFIX . "user SET iga_points = iga_points + $addpoints, iga_lastpointsdate = NOW() WHERE userid=$u";
$db->query_write($sql);
}

// Other Gamers
foreach ($addother as &$u) {
$sql = "UPDATE " . TABLE_PREFIX . "user SET iga_points = iga_points + $addpoints, iga_lastpointsdate = NOW() WHERE userid=$u";
$db->query_write($sql);
}

//Log the Entry
$sql = "INSERT INTO " . TABLE_PREFIX . "igpointslog (date,staffid,type,points,userids) VALUES (NOW(),'$staffid','add',$addpoints,'$allusers')";
$db->query_write($sql);

define('CP_REDIRECT', 'iga_points.php');
print_stop_message('iga_points_added');
}

/////////////////////// edit
if ( $_POST['do'] == 'edit' ) {

if ( !isset($_POST['username']) ) { rpm_print_stop_back('Please be sure every required field is filled out before submitting.'); }


$vbulletin->input->clean_array_gpc('p', array(
'username' => TYPE_STR
));

$edituser = $db->escape_string($vbulletin->GPC['username']);

$sql = "SELECT iga_points FROM " . TABLE_PREFIX . "user WHERE username = '$edituser'";
$result = $db->query_read_slave($sql);
$cpoints = mysql_result($result,0);

print_form_header($this_script, 'update');
print_table_header("Update ".$edituser."'s Points");
echo "<input type='hidden' name='username' value='$edituser'>";
print_label_row('Note:', 'Points must be between 0 and 400.');
print_input_row('Points', 'points', $cpoints);
print_submit_row('Update Points');
}

/////////////////////// Update
if ( $_POST['do'] == 'update' ) {

if ( empty($_POST['username']) OR !isset($_POST['points']) ) { rpm_print_stop_back('Please be sure every required field is filled out before submitting.'); }

$vbulletin->input->clean_array_gpc('p', array(
'username' => TYPE_STR,
'points' => TYPE_UNIT
));

$uuser = $db->escape_string($vbulletin->GPC['username']);
$upoints = $vbulletin->GPC['points'];

//$udate = date("Y-m-d");

if ($upoints < 0 or $upoints > 400) {
define('CP_REDIRECT', 'iga_points.php');
print_stop_message('iga_points_val_error');
} else {
$sql = "UPDATE " . TABLE_PREFIX . "user SET iga_points=$upoints, iga_lastpointsdate=NOW() WHERE username = '$uuser'";
$db->query_write($sql);

//Get the UserID
$idquery = $db->query_read_slave("SELECT userid FROM " . TABLE_PREFIX . "user WHERE username = '$uuser' limit 1");
if (!$idquery){ die('Invalid query: ' . mysql_error()); }
$userid[] = mysql_result($idquery,0);
$editid = serialize($userid);

//Log the Entry
$sql = "INSERT INTO " . TABLE_PREFIX . "igpointslog (date,staffid,type,points,userids) VALUES (NOW(),'$staffid','edit',$upoints,'$editid')";
$db->query_write($sql);

define('CP_REDIRECT', 'iga_points.php');
print_stop_message('iga_points_updated');
}

}

[kh99]

lol, I didn't know you could have a variable named $++++eck. Anyway, the only thing I notice is that you could change the type for cleaning mainusers and otherusers to TYPE_ARRAY_UINT (it looks like they are userids, right?).

[KGodel]

Ahh, when I looked up the cleaning stuff it only showed me TYPE_ARRAY. Also, that variable is bit check, the forums censored it because it has a naughty word.

[kh99]

Also, that variable is bit check, the forums censored it because it has a naughty word.

lol, that's a good one.

[KGodel]

Alright, so my display code is bunked somehow. Everything shows until right after the table headers are set. I checked the array via print_r and everything looks legit. It counts the entries correctly, so I'm not sure what the issue is. :(

<?php
/*================================================= =====================*\
|| ################################################## ################## ||
|| # ACTIVITY POINTS LOG BY DRMATH
|| ################################################## ################## ||
\*================================================ ======================*/

// ######################## SET PHP ENVIRONMENT ###########################
error_reporting(E_ALL & ~E_NOTICE);

// #################### PRE-CACHE TEMPLATES AND DATA ######################
$phrasegroups = array('style');
$specialtemplates = array('products');

// ########################## REQUIRE BACK-END ############################
require_once('./global.php');
require_once(DIR . '/includes/adminfunctions_template.php');

// ########################## USERNAME FUNCTION ###########################
function get_username($userid) {
$sql = "SELECT username FROM " . TABLE_PREFIX . "user WHERE userid = $userid";
$result = $db->query_read_slave($sql);
$username = mysql_result($result,0);

return $username;
}

// ######################## CHECK ADMIN PERMISSIONS #######################
if (!can_administer('canadminusers'))
{
print_cp_no_permission();
}

// ######################## GET PAGE START ################################
$page = filter_input(INPUT_GET,'page',FILTER_SANITIZE_NUMB ER_INT);

print_cp_header();
echo "<div class='pagetitle'>In Game Activity Points Log</div>";

// Get Points Logged //
$sql = "SELECT logid, staffid, type, points, userids, date FROM " . TABLE_PREFIX . "igpointslog";
$result = $db->query_read_slave($sql);
$logs = array();
while ( $a = mysql_fetch_array($result, MYSQL_NUM) ) {
$logs[] = $a;
}
mysql_free_result($result);

// Page Variables
if (($page == null) OR ($page == 1)) {$i = 0; $page = 1;} else {$i = (($page - 1) * 15);}
$totallogs = count($logs);
$maxpages = ceil($totallogs / 15);
$prevpage = $page - 1;
$nextpage = $page + 1;

////////////////////// Begin Log
echo "<br/><br/>";
echo "<table cellpadding='4' cellspacing='0' border='0' align='center' width='90%' style='border-collapse:separate' class='tborder' id='cpform_table'>
<tbody>
<tr valign='top'>
<td class='thead' colspan='6' align='right'> <a href='pointslog.php?'>[Restart]</a> </td>
</tr>
<tr>
<td class='tcat' align='center' colspan='6'>
<b>In-Game Points Log Viewer (page $page/$maxpages) | There are $totallogs total log entries.</b>
</td>
</tr>
<tr>
<td class='thead' align='left'>Log ID</td>
<td class='thead'>Staff Username</td>
<td class='thead'>Date</td>
<td class='thead'>Action</td>
<td class='thead'>Points</td>
<td class='thead'>User(s)</td>
</tr>";

////////////////////// Fill out 15 entries
foreach ($logs as &$l) {
if ($i < ($page * 15) ) {

// Set Variables
$logid = $l[0];
$staff = "<a href='user.php?do=edit&u=$l[1]'><b>" . get_username($l[1]) . "</b></a>";
if ($l[2] = "add") { $type = "Added Points"; } elseif ($l[2] = "edit") { $type = "Edited Points"; }
$points = $l[3];
$uarray = unserialize($l[4]);
$users = "";
foreach ($uarray as $u) {
$users .= " <a href='user.php?do=edit&u=$u'>" . get_username($u) . "</a>,";
}
$users = rtrim(trim($users), ",");
//$date = new DateTime;//
$date = $l[5];

// Get Odd or Even Row
if (($i % 2) == 1) { $class = "alt1"; } else { $class = "alt2"; }

echo "<tr valign='top'>
<td class='$class'>$logid</td>
<td class='$class'>$staff</td>
<td class='$class'>$date</td>
<td class='$class'>$type</td>
<td class='$class'>$points</td>
<td class='$class'>$users</td>
</tr>";
$i++;
}
}

////////////////////// End Log, Page-nation
echo " <tr>
<td class='tfoot' colspan='6' align='center'>";
if ($prevpage > 1) { echo " <input type='button' class='button' value='? First Page' tabindex='1' onclick='window.location='pointslog.php?page=1'> &nbsp; "; }
if ($page > 1) { echo " <input type='button' class='button' value='< Previous Page' tabindex='1' onclick='window.location='pointslog.php?page=$prev page'> &nbsp; "; }
if ($page < $maxpages) { echo " <input type='button' class='button' value='Next Page >' tabindex='1' onclick='window.location='pointslog.php?page=$next page'> &nbsp; "; }
if ($nextpage < $maxpages) { echo " <input type='button' class='button' value='Last Page ?' tabindex='1' onclick='window.location='pointslog.php?page=$maxp ages'>"; }
echo " </td>
</tr>
</tbody>
</table>";

print_cp_footer();
?>

After some tinkering, the code stops working at the line:
$staff = "<a href='user.php?do=edit&u=$l[1]'><b>" . get_username($l[1]) . "</b></a>";

I think the issue is with my function. NetBeans is telling me $db isn't defined, so my guess is this is the issue.

Fixed. The function wasn't working so I changed this bit up:
$staff = "<a href='user.php?do=edit&u=$l[1]'><b>" . fetch_userinfo($l[1])['username'] . "</b></a>";
if ($l[2] == "add") { $type = "Added Points"; } elseif ($l[2] == "edit") { $type = "Edited Points"; }
$points = $l[3];
$uarray = unserialize($l[4]);
$users = "";
foreach ($uarray as $u) {
// get_username($u) //
$users .= " <a href='user.php?do=edit&u=$u'>" . fetch_userinfo($u)['username'] . "</a>,";
}