Bug #7076 String incorrectly manipulated
Submitted: 7 Dec 2004 13:54 Modified: 20 Dec 2004 18:11
Reporter: Tal Kormas Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / J Severity:S3 (Non-critical)
Version:3.1.5-gamma OS:Linux (Linux (Redhat 9))
Assigned to: CPU Architecture:Any

[7 Dec 2004 13:54] Tal Kormas
Description:
I'm trying to replace 3.1.1-alpha with 3.1.5-gamma and I have problems. My application gets a password and shodowing itself, then send it to MySQL (4.0.18) for MD5 and tries to get a record:

			Connection con = sqlConPool.getConnection();
			stmt = con.createStatement();
			res = stmt.executeQuery("SELECT name,institutions,active,manager,reports FROM webusers WHERE username='"+user+"' and password=MD5('"+shadowPassword(password,user)+"')");
			if(res.next()) {		//There is

On 3.1.1-alpha I get a valid response from the server, but on 3.1.5 I get the following exception:

SQLPorblem: You have an error in your SQL syntax. Check the manual that corresponds to your MySQL server version for the right syntax to use near ''4^??x=?' at line 1 
com.mysql.jdbc.MysqlIO.checkErrorPacket(MysqlIO.java:2849) 
com.mysql.jdbc.MysqlIO.sendCommand(MysqlIO.java:1534) 
com.mysql.jdbc.MysqlIO.sqlQueryDirect(MysqlIO.java:1625) 
com.mysql.jdbc.Connection.execSQL(Connection.java:2280) 
com.mysql.jdbc.Connection.execSQL(Connection.java:2211) 
com.mysql.jdbc.Statement.executeQuery(Statement.java:1159) 
rep.servlets.Login.doPost(Unknown Source) 

I've also tried to replace the statement with PreparedStatement and set the shadowed password using setString and setBytes but then it's going to the DB without exception but never return the record.

How to repeat:
Create a demo program that open a connection, create a statement and then do md5 to some string and try to send a select command with the return string (remember to do new String(md.digest())) to MySQL.
[7 Dec 2004 14:26] Mark Matthews
You shouldn't be able to send a new String(md5.digest()) into a SQL query, as it would potentially have embedded binary data that may or may not need to be escaped.

The fact that this happened to work with an older version of the driver is just pure luck.

Please post a testcase where you demonstrate that it _doesn't_ work with prepared statements, i.e. 'doesn't return a record'....If you're using an older version of MySQL-4.1 (not 4.1.7, which is the latest), this might actually be a bug in the server that is related to how the optimizer works with prepared statements.
[7 Dec 2004 22:42] Tal Kormas
test case with prepare statement:

			Connection con = sqlConPool.getConnection();
			PreparedStatement stmt = con.createStatement("SELECT name,institutions,active,manager,reports FROM webusers WHERE username='"+user+"' and password=MD5(?)");
			stmt.setString(shadowPassword(password,user));
			res = stmt.executeQuery();
			if(res.next()) {		//There is

Thanks,

Tal
[7 Dec 2004 23:58] Mark Matthews
Tal,

There's still a couple of issues with your testcase,

> test case with prepare statement:
> 
> 			Connection con = sqlConPool.getConnection();
> 			PreparedStatement stmt = con.createStatement("SELECT
> name,institutions,active,manager,reports FROM webusers WHERE username='"+user+"'
> and password=MD5(?)");

Why isn't user a parameter here? You're going to get yourself into trouble with string concatenation, especially with security because of SQL injection, and even more so, since this is obviously code that deals with users/logins :(

> 			stmt.setString(shadowPassword(password,user));

We don't know what shadowPassword does here, and we don't know if it returns the same format of string as MySQL's MD5() function. Is there a reason you can't do the MD5() and such all in MySQL, so you're making an apples-to-apples comparison?

What version of the server are you using, again?

If you can clear that up, we might be able to help you out.
[8 Dec 2004 0:36] Tal Kormas
Matt,

Thanks for your reply. I'm using MySQL 4.0.18, and wish to move to MySQL 4.1 for future installations. The reason I'm doing ShadowPassword on the Application and then sends it to MD5 is for (a) backward compatibility (b) Shadow Password is more complicated then just MD5 and it allows us to validate more then just password. About the way to pass it to the application, how can I pass it to the application in such a way MySQL will recognize it? The test case for the PreparedStatement does work with the 3.1.1-alpha release. It is not an option for me to change this method of validating as we have customers running the other version and wish to upgrade.

Thank you,

Tal
[9 Dec 2004 21:20] Tal Kormas
Matt,

Ok, I've found it. It seems like setBytes works the same way in both drivers. I'm going to add code to change password for all the users that will log in to the system using the old password, and set it in the new method. Then in the future I will attach the new driver.

Thanks,

Tal