4

I can't use SQL parameters in Delphi, if I try to use them to protect my login form, I get the following error upon login

[0x0005] Operation not supported

The code I am using is :

SQLQuery1.SQL.Text := 'SELECT * FROM registered WHERE email= :Email'+
                      ' and login_pass= :Password';
SQLQuery1.ParamByName('email').AsString := Email;
SQLQuery1.ParamByName('password').AsString := Password;

SQLQuery1.Open; // Open sql connection
if SQLQuery1.recordCount >0 then form2.Show;

but it is not working, the code below works correctly but is it always unsafe :

SQLQuery1.SQL.Text := 'SELECT * FROM registered WHERE email="'+Email+
                      '" and login_pass= "'+Password+'"';

I am using TMySQLConnection and TMySQLQuery components, set ParamsCheck to True, and using the first code mentioned above which doesn't work, how to correct the problem!

Any suggestion or help would be appreciated.

Thank you

ain
  • 22,394
  • 3
  • 54
  • 74
Rafik Bari
  • 4,867
  • 18
  • 73
  • 123
  • 5
    +1 for insisting on using parameters, but note that storing passwords in the clear is also very bad. Use salted hashes instead: `SELECT * FROM registered WHERE email = :email AND passhash = SHA2(CONCAT(salt,:password),512)` – Johan Dec 27 '11 at 11:48
  • According to the docs TMySQLQuery does support parameters: http://www.microolap.com/products/connectivity/mysqldac/help/TMySQLQuery/Properties/Params.htm – Johan Dec 27 '11 at 11:51
  • @Johan the password variable contain the password already hashed in MD5 – Rafik Bari Dec 27 '11 at 12:03
  • 1
    MD5 is not a secure hash function and if you don't salt the hash a rainbow table will break it in seconds, see: http://stackoverflow.com/questions/401656/secure-hash-and-salt-for-php-passwords – Johan Dec 27 '11 at 12:12
  • You should 1) Add a salt 2) Hash n times. Hashing one time only is not very safe, rainbow tables made "reversing" hashes easy enough. –  Dec 27 '11 at 13:15
  • Moreover forget the * syntax but when you need a quick and **dirty** solution to get all columns when querying by hand. Because it will always retrieve all columns, if coupled with another attack it will return data you would not like to be seen, even columns that you may add **after** you wrote that query. It is always better to restrict the resultset to the columuns you actually need. If you need just to know if there is a record satisfying the WHERE condition don't return any column, for example a SELECT COUNT(*) will work and return no sensitive data. –  Dec 27 '11 at 13:19
  • On which line do you get the error? Open or the next one? You should be as much informative as you can, we are not in front of your screen. Maybe StackOverflow will allow it one day, but not now :) –  Dec 27 '11 at 13:28
  • Are you sure that MySQL supports named parameters? Have you tried using ? as a parameter mark and set indexed parameters? – Linas Dec 27 '11 at 14:09
  • Maybe the parameter names maybe case sensitive? Have you tried **Email** and **Password**? – stukelly Dec 27 '11 at 15:00
  • @idsandon, no need for a multi-pass hash, just use a secure hash function like SHA2. – Johan Dec 27 '11 at 19:25
  • @Johan: if you want to defeat rainbow tables, it's better to hash more than once. Some implementations hash hundred if not thousands of times. If your attacker doesn't know how many times the password were hashed, he would need much, much, much larger tables. Precomputed rainbow table for common algorithms, including SHA-2, are readily available, you just have to download them. Just check here for a generator: http://www.oxid.it/ –  Dec 27 '11 at 20:07
  • @ldsandon, that's what the salt is for. – Johan Dec 28 '11 at 07:33
  • @Johan: ask PGP, OpenSSL, WPA, etc. why do they employ multiple hashes instead of just relying on a salt. People often use short password, nad unless the salt is large, the salt itself can be guesses. I didn't say "don't use a salt". I said salt **and** hash n times. It takes little more time and increase security a lot. Why avoid it? –  Dec 28 '11 at 16:58
  • @ldsandon, the reason to avoid multiple passes is that is does not give you any edge over an attacker. Lets assume a single pass takes 10 milliseconds. A 1000 passes will take 10 seconds. These 1000 passes will make it a 1000 times harder for an attacker to crack your hashes. However, you will have to invest in extra hardware because you can only handle 1 request per 10 seconds. If you use a truly random salt between 000 and 999 you don't spend any extra time, but still make it a 1000x harder to crack you. Much better tradeoff. – Johan Dec 28 '11 at 17:45
  • @ldsandon, Note that it is much easier for an attacker to get extra horsepower, he just employs more zombies, you on the other hand have to spend real money on real hardware, all the time. Every time you authenticate a user you will have to do your repeat hashes, wasting CPU-time and money. If you use a salt, a secure hash function and a sane implementation you don't spend any extra money, but it is trivially easy to make the cracking a 1,000,000 times harder. Not so with repeat hashing, it gives a marginal improvement (1000x) for a **lot** of effort. – Johan Dec 28 '11 at 17:49
  • @Johan: just your way to define a salt range shows you knowledge of security. –  Dec 28 '11 at 21:57

2 Answers2

3

Check the help for "RecordCount". It may raise an exception if the dataset can't determine how many records are returned. What if you remove it and simply check if the dataset not IsEmpty?

2

Use salted hashes for your password check
Storing a unencrypted password in a database is a no-no.
Use a salted hash instead:

SELECT * FROM registered WHERE email = :email 
AND passhash = SHA2(CONCAT(salt,:password),512)

You can store the passhash in the DB by doing: 

INSERT INTO registered (email, passhash, salt) 
VALUES (:email, SHA2(CONCAT(:salt,:password),512), :salt)

The salt does not need to be truely random, but it does need to be somewhat random and different for each user.

Johan
  • 74,508
  • 24
  • 191
  • 319