sql injection worked on a own script...

[loop]

Hi All
i'm trying to figure out the best / savest way to process a $_GET / $_POST variable in a query. The Parameter is a string so i cannot make it integer only.

I saw a few places where used
Text::input or Text:prepare

at the moment i use:

Code: Select all

...AND attribute_id = '" . tep_db_input(Text::input($_POST['active_field_id'])) . "'' ....

is there a better way to go?

unfortunately i had a attack which i saw in the logs....so now i try to fix every hole...

Code: Select all

SELECT products_id, count(*) as count_fulfilled FROM techspecs_value WHERE (group_id IN (if(now()=sysdate(),sleep(15),0)) AND (attribute_id = 'B168_M168_ZUSATZFUNKTIONEN' AND attribute_value_text = 'Bluetooth') ) GROUP BY products_id;

the fix i tried tep_db_input(Text::input( seems to help for this particular injection but i'm not sure if this is the best way, thanks in advance

[ecartz]

It would be better to use $db->escape than tep_db_input, as $db->escape will still be there after I remove the tep_ functions.

But in terms of SQL injection, either would work. The Text::input prevents HTML/XS injections and removes extraneous whitespace; it has nothing to do with SQL. Still good to use it--just don't expect it to do anything against SQL injections unless they have a < or > in them. Text::prepare just removes extraneous whitespace and should not be used with customer input, only with known good input from authenticated admins. All customer input should go through Text::input before being used.

It won't help you here, but if you are entering integers, you can cast to int instead of using $db->escape.

[loop]

thank you ecartz

so $db->escape + Text::input you would suggest for customer_inputs to be safe, right? (and of course, if it's a int cast to int)

i saw in many places this, and i'm not sure if this is also because of this problem (sprintf):

Code: Select all

 $hooks_query = tep_db_query(sprintf(<<<'EOSQL'
SELECT hooks_site, hooks_group, hooks_action, hooks_code, hooks_class, hooks_method
 FROM hooks
EOSQL
    , tep_db_input(tep_db_prepare_input($file))));

[ecartz]

The sprintf is more to make it easier to read and organize. It does help slightly when working with integer IDs, but it's not really a security enhancement.