Am Freitag, den 20.08.2010, 21:54 -0700 schrieb Yawar Khan:
> Chris, you identified a possible sql injection in my code and declaring it a
> very bad piece of code. Despite the fact that jdbc does not allow more than 1
> query on this execute function and I am doing fields validation before
> submission of the form.
>
>
> Is there another genuine threat or bug that you identified and would like to
> share? Please do, I am sharing the udac source code as well,
>
>
> Wesley you comments are also welcome; somebody also asked that what will
> happen
> in case udac.login throws an exception, well exception handling is inside
> this
> class. Sorry but i missed that email so i am unable to name that gentleman
> friend.
>
> package org.mcb.services;
>
> import java.text.*;
> import java.util.*;
> import java.sql.*;
> import javax.servlet.http.HttpSession;
>
> public class udac
> {
> static Connection currentCon = null;
> static ResultSet rs = null;
This seems to be really problematic. Having ResultSet and Connection
shared by many users is a bad idea.
Imagine what happens when two requests come in at the same time:
Request A Request B
login(beanA)
|
currentCon=new Connection()
| login(beanB)
| |
| currentCon=new Connection() # BOOM you are
overwriting the class wide variable currentCon.
Same thing can happen to rs too. So better place currentCon and rs as
method variables inside of login.
>
> public static userbean login(userbean bean) {
> //preparing some objects for connection
> Statement stmt = null;
> String userid = bean.getUserId();
> String password = bean.getPassword();
> String epass = null;
> String name = null;
> String user_id = null;
> String role_id = null;
> String branch_code = null;
> String last_login = null;
> String role_desc = null;
> try{
> epass = passwordservices.getInstance().encrypt(password);
> //passwordservices is a class which has functions to ecrypt a
> string and return back the string.
> }catch(Exception e){
> System.out.println(e);
I find it very useful to use a logging framework for reporting errors.
And adding information about the state in which the error occured might
help finding the root cause more easily.
> }
> String searchQuery = "SELECT a.USER_ID,a.NAME, a.BRANCH_CODE,
> a.PASSWORD, a.LAST_LOGIN_DATE, a.ROLE_ID, b.ROLE_DESC FROM LOGIN_INFORMATION
> a,
> ROLES b WHERE a.ACTIVE = 'A' AND a.ROLE_ID = b.ROLE_ID ";
> searchQuery = searchQuery + "AND LOWER(a.USER_ID) = LOWER('"+
> userid
> + "') AND a.PASSWORD = '"+epass+"'";
If your are using prepared Statements with parameters, you don't have to
worry, if someone has forgotten to check those parameters for
sql-injection. But you were told so already.
Bye
Felix
> try{
> //connect to DB: connectionmanager is a class which contains
> connection functions
> currentCon = connectionmanager.scgm_conn();
> stmt=currentCon.createStatement();
> rs = stmt.executeQuery(searchQuery);
> boolean hasdata=false;
> while(rs.next()) {
> hasdata=true;
> name = rs.getString("NAME");
> user_id = rs.getString("USER_ID");
> branch_code = rs.getString("BRANCH_CODE");
> role_id = rs.getString("ROLE_ID");
> last_login = rs.getString("LAST_LOGIN_DATE");
> role_desc = rs.getString("ROLE_DESC");
> bean.setName(name);
> bean.setUserId(user_id);
> bean.setBranch(branch_code);
> bean.setRole(role_id);
> bean.setLastLogin(last_login);
> bean.setRoleDesc(role_desc);
> bean.setValid(true);
> }
> if(!hasdata) {
> System.out.println("Sorry, you are not a registered user!
> Please sign up first "+ searchQuery);
> bean.setValid(false);
> }
> }catch (Exception ex){
> System.out.println("Log In failed: An Exception has occurred! "
> +
> ex);
> }
> //some exception handling
> finally{
> if (rs != null) {
> try {
> rs.close();
> } catch (Exception e) {}
> rs = null;
> }
>
> if (stmt != null) {
> try {
> stmt.close();
> } catch (Exception e) {}
> stmt = null;
> }
>
> if (currentCon != null) {
> try {
> currentCon.close();
> } catch (Exception e) {
> }
>
> currentCon = null;
> }
> }
> return bean;
>
> }
> }
>
> ysk
> -----Original Message-----
> From: Christopher Schultz [mailto:ch...@christopherschultz.net]
> Sent: Friday, August 20, 2010 3:43 AM
> To: Tomcat Users List
> Subject: Re: Sessions mix-up on Tomcat 6.0.26 on Linux
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Wesley,
>
> On 8/19/2010 5:04 PM, Wesley Acheson wrote:
> > Maybe its just be but I still don't see where uadc is declared or even
> > imported.
>
> ...or even used.
>
> I'm guessing that the bad code exists outside of this login servlet.
>
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (MingW32)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkxts1YACgkQ9CaO5/Lv0PBitwCeMXvEXLi1L9rnLmTVP4nofIGH
> NkAAnj9DTqFLwLAYxb2MQuI6v6ckVcYm
> =DR0I
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: users-h...@tomcat.apache.org
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
