Code Review - Help needed resurrecting an abandoned modification

[PhuriousGeorge]

Hey guys, I've been working to update a modification that was created last year that required some updates and tweaks the community has provided, however I seem to be having issues with installing the package through the package manager. I was receiving database errors initially, but as I tweaked more and more within the db_install.php file, I'm now getting an error: "Hacking Attempt..." when attempting to install.

I'm having these issues whether I attempt to install the original version or my updated version.

I was curious if anyone would be nice enough to review the file, as I'm rather new to modifications and php itself.

The issue itself appears to be within db_install.php and I'm assuming the creation & population of the tables that need to be created. I attempted to look at a few other mods as examples, but didn't want to break even more.

For ease of access, here's some of the code from db_install.php:

Koodi [Valitse] Laajenna

// Set the below to true to overwrite already existing settings with the defaults. (not recommended.)
$overwrite_old_settings = false;

//added
global $smcFunc, $db_prefix;

/******************************************************************************/

// If SSI.php is in the same place as this file, and SMF isn't defined, this is being run standalone.
if (file_exists(dirname(__FILE__) . '/SSI.php') && !defined('SMF'))
require_once(dirname(__FILE__) . '/SSI.php');
// Hmm... no SSI.php and no SMF?
elseif (!defined('SMF'))
die('Hacking attempt...');

// Install charges
$chargeTable = "CREATE TABLE IF NOT EXISTS {$db_prefix}EFTCharges` (
`typeID` int(11) NOT NULL,
'typeName` varchar(255) NOT NULL
) ENGINE=MyISAM;";

$result = $smcFunc['db_query']('',$chargeTable);

// Uh-oh spaghetti-oh!
if ($result === false)
echo '<b>Error:</b> Charge table failed!';
else
echo 'succeeded';

$charges = "
INSERT INTO {$db_prefix}EFTCharges (`typeID`, `typeName`) VALUES
(178, 'Carbonized Lead S'),
-------SNIP------
(32995, 'Copper Firework CXIV')";

$result = $smcFunc['db_query']('',$charges);

// Uh-oh spaghetti-oh!
if ($result === false)
echo '<b>Error:</b> Charge INSERT failed!';

I've attached:
db_install.php - My modified file I'm having issues with

Thanks for any assistance!
-PG

[Illori]

since the mod has no license you are not allowed to distribute it or update it for others to download as a package. your attachments have been removed minutes the php file.

[PhuriousGeorge]

Apologies, I thought the "Feel free to alter the code. I take no responsibility for damage to your board." in the readme.txt from the original author was enough 

[TheListener]

Which mod is it anyway?

[Illori]

no that is not a license, a clear license has to be stated.

[PhuriousGeorge]

Which mod is it anyway?

It's a mod that was never officially posted here. It's for displaying a Ship's fitting in-post from the game EVE Online by parsing an export from in-game or an external ship fitting tool.

When it works, it should look like:

no that is not a license, a clear license has to be stated.

Fair enough, I'll contact the originator and put one together.

[PhuriousGeorge]

So anyways, no ideas on the issue?

[The Wizard]

Hello:

You need to take the time and contact the author of the mod and get written permission from them that you can change their work. Once you have that in hand and post it here your good to go and then people will help.

I know this sounds silly, but look at it this way - Imagine you just wrote a book, and posted it, and then someone else who you do not know takes your book and rewrites the ending. Then they post it and has the nerve to say it was now their creation.

That is what your asking us to do, and no good programer will do this.

I know your thinking, but it's just a small mod and I need the changes...

The fact is their are no small mods.  The people who write and create mods work very hard on them spending hours, days, weeks, months and even years on them. My small approve mod monthly banners and holidays took me over 8 months to design and get approved. The one I'm working on now could take me a year or more.

In short respect the creator of the work and just ask - 9 out of 10 times you will get permission - you might even get them to help.

Wiz

   

[emanuele]

Hey guys, I've been working to update a modification that was created last year that required some updates and tweaks the community has provided, however I seem to be having issues with installing the package through the package manager. I was receiving database errors initially, but as I tweaked more and more within the db_install.php file, I'm now getting an error: "Hacking Attempt..." when attempting to install.

That is usually a warning at the beginning of most of the files (usually all those that should not be accessed directly, but through index.php or SSI.php). The install you attached doesn't include such string, so it's not the cause of the error.
Most likely is something else.

ETA: and you should consider reading a bit http://wiki.simplemachines.org/smf/$smcFunc and "convert" the queries to $smcFunc "properly".

[onepiece]

I'm sure the original poster has understood that he can't share the modified code by now and he can modify it however he likes and use himself.

By the way, the reason why you're getting that "Hacking attempt..." is mostly probably because you have semicolons in your queries. I can see at least one right away in the code you quoted. (the first query) You just need to remove those. You don't need them.

[Arantor]

It's almost certainly not the semicolons, actually, but more likely the raw apostrophes. There are proper facilities for inserting rows of data, namely $smcFunc['db_insert'].

[onepiece]

Incorrect use of apostrophes are a different issue. SMF won't complain about them with a "Hacking attempt..." error message. SMF considers semicolons and stuff like unions or sub-selects dangerous and complain about them in that manner if they appear in the queries, unless $modSettings['disableQueryCheck'] is enabled.

[Arantor]

SMF won't complain about them with a "Hacking attempt..." error message.

Yes it will. I draw your attention to line 249 of Subs-Db-Mysql.php.

Koodi [Valitse] Laajenna

if (empty($modSettings['disableQueryCheck']) && strpos($db_string, '\'') !== false && empty($db_values['security_override']))
smf_db_error_backtrace('Hacking attempt...', 'Illegal character (\') used in query...', true, __FILE__, __LINE__);


And in the very first query in the above code I see bare apostrophes.

[emanuele]

* emanuele should actually read the code instead of give bad advices based on a quick scan and some assumptions...

[Arantor]

I know the feeling, emanuele... I've been working with this stuff far too long though. I had to double check exactly what it did because 1.1.x behaviour is slightly different to 2.0's behaviour. But the conversion of everything everywhere to htmlspecialchars with ENT_QUOTES should be a fairly big giveaway