Wallet Manager Security Issues

If you have been following this blog for a while, you know that my corpmate James and I have been working on a Wallet Manager site to help manage our Eve ventures. Over time it has grown into our all-encompassing-project-management-thing which now has a trading, manufacturing, invention, and cost analysis sections.

I wanted to disclose why this darn thing is not open to the public as the majority of the feedback that I have been hearing has been, “awesome, now when can I use it!?”

We have not made the site public because of security issues, specifically due to the numerous SQL injection abilities in our code.

Here is a common function that we use that takes the typeID of an item and returns its name. We use this so when we display a Cap Recharger II for example, you can see the name of the item and not just the ‘2032’ number identifier that is easier to work with from a programmability standpoint.

This PHP function retrieves the item name from an input of its typeID.

public function getName($typeID)
{
$sql = ‘SELECT typeName FROM invTypes WHERE typeID = ‘.$typeID.’‘;
$connection=Yii::app()->db;
$command=$connection->createCommand($sql);

//Run the query
$results = $command->query();
$itemName = $results->read();

return $itemName[‘typeName’];
}

The database query is highlighted in green and the terrible part has been highlighted in red.

What you are seeing is a database query that is fed a non-sanitized input. Good programmers will take the $typeID variable and sanitize it before putting it into the SQL query. A common check is to limit the variable to only have characters such as A-Z and 1-3 characters. This check will not allow any special characters such as  : ; ‘ ” $ that are used for SQL operations to be allowed in the query.

With our current function with the unsanitized input variable, you can plug in all sort of things into the query. You could inject code in place of the variable to read, drop, and modify the database,  something we obviously don’t want happening.

Sadly around half of our function were written in this fashion in order to get the pages up and working. Because it has been an internal project, the focus has been on the aesthetic result and not the security of the code behind it. If we were to release it to the public we would have to go over each function and check to make sure that it is secure.

Let me quote CCP and say Soon(tm) for the release.