[RESOLVED] Stop hackers by a trickery!

Zpixel

after limiting input data by size or type, i think, the other way is to eliminate some keywords:

//stop hackers
$baned=Array('delete','drop','insert','replace','select');
foreach($_REQUEST as $item){
$item=strtolower($item);
foreach($baned as $x)
if($x==$item)
exit;
}

i have put the code above in my pages and it works. How can i make it better? is it vulneruble for a nimble hacker? what other words to include in the list ($baned I mean) ?
thanks.

NogDog

I don't see the need for that, assuming the purpose is to stop database attacks via SQL injection. If you properly escape all inputs used in your DB queries, such attacks cannot work no matter how many of those words they use.

laserlight

The problem is that those words are common and have valid usage. Besides, even a more sophisticated script that checks for the structure of SQL statements could end up banning an honest user posting about an SQL statement.

As NogDog noted, the correct solution is to properly escape input to your database, whether by using prepared statements or by using an appropriate escaping function.

Zpixel

even if nothing in my pages need to send sql information using GET or POST?

the code above when placed in a header.php page, eliminats all unwanted word.
i 'm not sure about my sql injection precautions, so i used the code above to relieve myself at once.

bradgrafelman

The code above doesn't eliminate all unwanted input nor does it protect against SQL injection attacks. So, in short, if you want our rating on its effectiveness on a scale of 1 through 10 (10 being bulletproof, 1 being as solid as a screen door), it's going to get about a 3.

Moral of the story: get rid of all of that nonsense and use [man]mysql_real_escape_string/man (or something similar, e.g. prepared statements, whatever). It's a) cleaner, b) less code, and c) actually going to help secure your application against SQL injection.

laserlight

Zpixel wrote:
the code above when placed in a header.php page, eliminats all unwanted word.

If I read your code above correctly, it does no such thing. What is does is check if a banned word matches a given input, but that certainly would not match "DROP TABLE Students", much less "TRUNCATE TABLE Students". bradgrafelman's rating of 3 applies if you changed it to check if the given input contains a banned word; as it stands I would rate it a 0.

Zpixel

only 3 scores? why? i revised it.

//stop hackers
$baned=Array('delete','drop','insert','replace','select','truncate');
foreach($_REQUEST as $item){
$item=' ' . strtolower($item);
foreach($baned as $x)
if(strpos($item,$x))
exit;
}

fortunately, my website language is not English and i can eliminate those words without any problem. that's your right. it is weak, but easy to use. not any other cosideration is needed. i give it 4 or 4.5. the problem is that it dosen't look like a pro piece of code. it is simple, but works.

A question. can this code be hacked for inserting sql strings?

thanks.

laserlight

Zpixel wrote:
only 3 scores? why?

Because you did not account for TRUNCATE, and now, what about UPDATE?

Zpixel wrote:
fortunately, my website language is not English and i can eliminate those words without any problem.

In that case, this idea is workable, but only as a secondary defense. The primary defense remains correctly escaping your database input.

Zpixel wrote:
A question. can this code be hacked for inserting sql strings?

Yes, but whether or not the SQL injection amounts to anything depends on whether your "banned words" list correctly identifies what operations may be performed. You may still have sensitive information leakage if the attacker writes an invalid SQL statement and it is feedback to him.

Zpixel

laserlight, i have included truncate in my previous post.

well, thanks. what about using database table names instead of these common words?

for example my db table names are: sfk_users , sfk_ads , sfk_address_book , ...

we can eliminate them without making any restriction to users. no one needs to use sfk_users or sfk_ads as input. doesn't it?

Zpixel

//stop hackers
$baned=Array('delete ','drop ','insert ','replace ','select ','truncate ','update ');
foreach($_REQUEST as $item){
$item=' ' . strtolower($item);
foreach($baned as $x)
if(strpos($item,$x) && strpos($item,'mysql_query'))
exit; // or redirect
}

laserlight

Zpixel wrote:
i have included truncate in my previous post.

Yes, but you did not include it from the start, and perhaps you do not even need to include it, e.g., if the database account is not allowed to use truncate. The point here is that trying to filter based on a blacklist of words is prone to errors of omission.

Zpixel wrote:
what about using database table names instead of these common words?

Sure you could, but again, beware of errors of omission. What happens if your database schema changes to add a new table but you (or some future maintainer) forgets to update the blacklist, and this knowledge somehow gets into the hands of an attacker? There are ways of obtaining the list of table names programmatically, but that would be overhead. You may find it even simpler to target "--", which is used to comment out the parts of an SQL statement following an SQL injection.

What I am concerned about is that not once have you acknowledged what we have been saying about properly escaping your database input. Have you done so?

Zpixel

yes, sure. i have filterd db input by common methods.
and, it's your right. every thing may be forgotten.
you were absent for a short time laserlight. goodluck.

so thanks again for all the answers.

Zpixel

نیازمندیها, ارسال آگهی, تبلیغات, تبلیغات رایگان

djjjozsi

what is your code doing if i write alter to modify your tables.

Krik

Not to add fuel fire here. But when there are foolproof methods built in to PHP why go about reinventing the wheel.

Not that I know a lot about sql injections but what if the value of $item was

'') {/*some code here*/}

or some other variant so as to bypass the code that is checking it.

Sorry, I must through my vote to [man]mysql_real_escape_string/man and other built in methods in PHP

Zpixel

djjjozsi , include the keyword -alter- and other harmful words in the $ban array.

mysql_real_escape_string() is good but enough?
there was a contest in one of America cities to hack the state police website. One hacker did the job in less than some minutes.

using common ways is very needed, but try to invent somthing as your personal way which is unknown to hackers.

if(strpos($item,$x) ) => if $item==''){mysql_query("delete ...")} => it can not defeat the code above.

i have put a sample page in the address bellow:
http://www.aladdinmobile.com/ecardx/test.php

this is the code of the file that makes a query from db:

mysql_query("SELECT * FROM orders WHERE id='$id'");
the table name is => orders

send your GET data to manipulate db. db user has a drop right i think.
if you coud be successful to hack it, please inform me.

not any common way such as adslashes or mysql_real_escape_string() is used.

bradgrafelman

Zpixel wrote:
mysql_real_escape_string() is good but enough?

Yes, actually, it is. It escapes characters that would otherwise interrupt the query or change its behavior.

Zpixel wrote:
using common ways is very needed, but try to invent somthing as your personal way which is unknown to hackers.

That's absurd; the "common ways" you're speaking of are ones that have been developed and used for years. Since bank's commonly keep money in a secured vault, maybe they should try to trick bank robbers by hiding it in large trash bags near the trash can instead?

Zpixel wrote:
if(strpos($item,$x) ) => if $item==''){mysql_query("delete ...")} => it can not defeat the code above.

Assuming you're correct, awesome; you prevented one situation out of many. Now as others chime in and mention ideas that you didn't think of, you admit that it doesn't work and then you modify the code to protect yourself against that situation. Meanwhile, those of us sitting here using mysql_real_escape_string() were already protected against all of the situations discussed already plus several more. It's less code and protects against more cases than your code does, so I fail to see why anyone would want to switch.

Can you tell us why you think it's better to have

$baned=Array('delete ','drop ','insert ','replace ','select ','truncate ','update ');
foreach($_REQUEST as $item){
$item=' ' . strtolower($item);
foreach($baned as $x)
if(strpos($item,$x) && strpos($item,'mysql_query'))
exit; // or redirect
}

than

mysql_real_escape_string($data);

Zpixel

mysql_real_escape_string() is a function with internal programing. maybe some lines of code.
the six lines above could be packed in one function. therefore, the result is the same. both functions act properly. but there is a merit in the new function. we can use it for example in the header page(header.php). this page is included in all the other pages using 'include()'.

so we can use this function only one time and secure all the pages, no need to use mysql_real_escape_string() for every query or input data. (of course i have used mysql_real_escape_string and addslashes before any query)

this integration (let us laugh a bit) can be remarkable. as you know one of the idea behind creating CLASSES was to ease actions because of integration.

Dont oppose to new ideas just because of they are new or even absured. i'm here to learn.

all the other coders gave me a 3, give me a mark even 0, but judge fairly.

bradgrafelman

Zpixel wrote:
therefore, the result is the same. both functions act properly.

The result isn't the same because both functions don't do the same thing. Yours looks for specific keywords to reject, whereas [man]mysql_real_escape_string/man prevents the characters that allow attackers to inject their own statements into the query string. Things such as backslashes, quotes, and it even prevents certain characters that might break the query (e.g. ones found in binary files).

Zpixel wrote:
(of course i have used mysql_real_escape_string and addslashes before any query)

[man]addslashes/man should never be used to prepare data for a SQL query.

Again, the only reason I'm saying your code shouldn't be used is because it still misses several cases that will alter or break a SQL query, ones that [man]mysql_real_escape_string/man for example already covers.

Zpixel

addslashes() should never be used to prepare data for a SQL query.

it was valuable to me.
thanks again.