tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Eggers <its_toas...@yahoo.com>
Subject Re: [OT] Sessions mix-up on Tomcat 6.0.26 on Linux
Date Thu, 19 Aug 2010 19:58:48 GMT
Client side validation is for convenience and user feedback. Server side 
validation is still required. Nothing requires a user to use a browser, or to 
not use extension like Fiddle or Tamper to play with the information once it's 
passed your validation scripts.

. . . just my two cents.

/mde/



----- Original Message ----
From: Yawar Saeed Khan/ITG/Karachi <yawar.saeed@mcb.com.pk>
To: Tomcat Users List <users@tomcat.apache.org>
Sent: Thu, August 19, 2010 12:27:08 PM
Subject: RE: [OT] Sessions mix-up on Tomcat 6.0.26 on Linux

thanks for your constructive comments, as I mentioned that "bad, bad, bad" code 
is out. no longer in the application...

your comments on my current code tells me that this code is not bad, but I 
should check out tomcat's container managed logins... right?

plus I would like to mention that I have client side form validations (js) to 
stop query busters.

________________________________

From: Christopher Schultz [mailto:chris@christopherschultz.net]
Sent: Thu 19-Aug-10 11:01 PM
To: Tomcat Users List
Subject: Re: [OT] Sessions mix-up on Tomcat 6.0.26 on Linux



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yawar,

I'm marking this as off-topic for /your/ request. I just have some
comments for you. Take them or leave them.

On 8/19/2010 11:53 AM, Yawar Saeed Khan/ITG/Karachi wrote:
> Ok, let me share my source code with you...
>
> my index.jsp page has a html form which submits the form data to a servlet 
>called loginmanager.
>
> this is the code inside doPost function;
>
>         try {
>
>              userbean user = new userbean();   // usebean is a class the has 
>setter and getter functions for user attributes
>
>              user.setUserId(request.getParameter("txt_userid"));
>
>              user.setPassword(request.getParameter("txt_pass"));
>
>              user = udac.login(user); //udac is a class that has data access 
>functions, login function takes user object and checks its existence in db and 
>sets isValid attribute for that user

Not using Tomcat's container-managed login? Any particular reason why
not? It's quite easy to configure and has the added benefit of being
properly tested.

>              if (user.isValid()){
>
>                   HttpSession session = request.getSession(true);
>
>                   session.setAttribute("user_id",user.getUserId());
>                   session.setAttribute("user_name",user.getName());
>                   session.setAttribute("role_id",user.getRole());
>                   session.setAttribute("role_desc", user.getRoleDesc());
>                   session.setAttribute("last_login", user.getLastLogin());

Why not session.setAttribute("user", user)?

>                   response.sendRedirect("main.jsp"); //logged-in page

That should be:

response.sendRedirect(request.getContextPath()
                    + response.encodeRedirectURL("/main.jsp"));

>              }else{
>
>                   response.sendRedirect("index.jsp?user="+user.isValid()); 
>//revert back to login page

That should be:

response.sendRedirect(request.getContextPath()
                    + response.encodeRedirectURL("/main.jsp")
                    + "?user="
                    + java.net.URLEncoder.encode(user.isValid()));

It always helps to format and encode things properly.

>              }
>
>         } finally {
>
>             out.close();
>         }

What is "out"?

> Previously i had tried a simple way; my index.jsp file called itself on form 
>submit, below code was in index.jsp (no servlet etc);
>
>  //after form is submitted
>
> String query = "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 ";
>
>  query = query + "AND LOWER(a.USER_ID) = LOWER('"+ 
>request.getParameter("txt_userid") + "') AND a.PASSWORD = '"+ epass +"'";
>
>         boolean hasdata=false;
>
>         java.sql.ResultSet rs = connection.executeQuery(query);

Wow: this is a SQL injection attack just waiting to happen. What happens
if I submit the txt_userid request parameter as "') OR 1;" or, even
better, "'); DELETE FROM LOGIN_INFORMATION;" or some other evil thing?

I believe that certain JDBC drivers will not execute more than one query
per executeQuery() call, but you can't really count on that. It's easy
to use a PreparedStatement and just do it properly: poof! SQL injection
attacks are a thing of the past (unless the driver is broken, but they
test those things very well).

Also, most SQL databases perform case-insensitive string comparisons, so
your LOWER(a.USER_ID) = LOWER(...) can probably be simplified. Note that
it also means you likely have case-insensitive passwords (though you
haven't shown us what "epass" is -- is could have been hashed.

>  while(rs.next()) {
>
>             hasdata=true;
>
>             session.setAttribute("user_id",rs.getString("USER_ID"));
>
>             session.setAttribute("user_name",rs.getString("NAME"));
>
>             session.setAttribute("branch_code",rs.getString("BRANCH_CODE"));
>
>             session.setAttribute("role_id",rs.getString("ROLE_ID"));
>
>             session.setAttribute("role_desc",rs.getString("ROLE_DESC"));
>
>             
session.setAttribute("last_login",rs.getString("LAST_LOGIN_DATE"));
>
>             upsql = "UPDATE LOGIN_INFORMATION SET LAST_LOGIN_DATE = SYSDATE 
>WHERE USER_ID = '"+ rs.getString("USER_ID") +"'";
>
>             int up = connection.executeUpdate("UPDATE LOGIN_INFORMATION SET 
>LAST_LOGIN_DATE = SYSDATE WHERE USER_ID = '"+ rs.getString("USER_ID") +"'");
>
>             int audit_insrt = InsertAuditEntry("F001", (String) 
>session.getAttribute("user_id"), (String) session.getAttribute("branch_code"));
>
>             response.sendRedirect("main.jsp");

How many redirects do you end up sending? Hopefully, only one. But this
code is bad, bad, bad. It makes me wonder what other nuggets can be
found in your code.

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ 

iEYEARECAAYFAkxtY30ACgkQ9CaO5/Lv0PA1pgCcDe1cNVlaqRNlWAbyQVybng4X
OpUAn3ab9KDdsYvVGYzQmoeB871SgUqp
=eEX2
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org




This E-mail is confidential. It may also be legally privileged. If you are not 
the addressee you may not copy, forward, disclose or use any part of it. If you 
have received this message in error, please delete it and all copies from your 
system and notify the sender immediately by return E-mail. Internet 
communications cannot be guaranteed to be timely, secure, error or virus-free. 
MCB Bank does not accept liability for any errors or omissions.


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


      


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Mime
View raw message