Menu
▾
▴
Re: [WebMacro-user] Session integrity issue?
|
From: Keats K. <ke...@xa...> - 2006-02-21 03:28:59
|
Nikolaos,
I don't know S2J, but I assume it's an O/R mapping tool. I would focus
my attention on the following line of code:
Users[] foundUsers = UsersManager.getManager().loadByWhere(
"where USE_EMAIL=\'" + login + "\' AND USE_PASSWORD=\'"
+ password + "\'", connection);
Is this invoking customized code? Is loadByWhere() synchronized, or is
there some other synchronization mechanism here?
(Btw, unless the S2J protects against it, this code could be vulnerable
to a SQL-injection attack. Make sure you check the userid and password
for apostrophes or other special characters that might trip up your RDBMS.)
Hope this helps.
Keats
Nikolaos Papadakis wrote:
> Hi all,
> I am sending a cut (but indicative---I hope) version of the core
> servlet I am using.
> The userId variable is set into the session by the validateUser method.
> Various other methods have been omitted in this version for
> readability reasons.
> Hope it is clear.
>
> Thank you in advance
> Nikolaos Papadakis
>
> P.S. Just for the record the configuration I am using includes:
> SuSe ver 10.0
> Java 1.4.2
> Tomcat 5.0.30 in conjunction with Apache ver 2.0.53.
> I also use some customized version of S2J to “objectize” tables and
> relations in the db.
>
>
>
>
>
> Keats Kirsch wrote:
>
>> It's not possible to tell from the code snippet what is going wrong.
>> The concurrency issue that Lane is referring to is fairly obscure
>> (only affecting the #include directive with dynamic template names if
>> I recall correctly) and not relevant here.
>>
>> The code as shown looks OK, but we can't see how the USERID is stored
>> in the session, or how the user data is stored and retrieved based on
>> this USERID. The bug could be in the data access layer or the
>> session management subsystem (which depends on your Servlet
>> container), or in some caching mechanism. It is unlikely, but not
>> impossible that this could be a WebMacro bug. If I had to guess, I
>> would suspect that the code which actually retrieves the user data is
>> not properly synchronized.
>>
>> Send us a bit more information about your application, and maybe we
>> can help you track this down.
>>
>> Keats
>>
>> Lane Sharman wrote:
>>
>>> Hi Nikolaos.
>>>
>>> This is most disturbing to hear about.
>>>
>>> First, there is a known concurrency issue with WM in 2.0 and
>>> possibly in prior releases. If you look at the mail thread dating
>>> back for 45 days, you will definitely see it.
>>>
>>> My concern is that the webmacro instance, shared, and the context,
>>> not shared, is improperly orchestrated for concurrency. The webmacro
>>> instance is local to WMServlet. You might consider synchronizing
>>> access to this variable as a trial fix in your local calls below.
>>> This will introduce some serialization of web processing but it
>>> might help to verify the solution.
>>>
>>> One thing you might consider trying is to use a different cache
>>> handler. A few years ago, I wrote one and it is a part of the
>>> distro. I have never had a problem using this cache handler.
>>>
>>> Lane
>>>
>>> --- Nikolaos Papadakis <nk...@cc...> wrote:
>>>
>>> From: Nikolaos Papadakis <nk...@cc...>
>>> Date: Sat, 18 Feb 2006 10:50:33 +0200
>>> To: web...@li...
>>> Subject: [WebMacro-user] [WebMacro-user
>>>
>>> Hi all,
>>>
>>> I am using webmacro to develop a web-based application.
>>> Things seemed to work fine till yesterday. A customer complained
>>> that he
>>> managed to “see” the account of another customer when he logged-in
>>> using
>>> his own credentials (he send me a screen shot of the “view”).
>>> By inspecting the log file I found that both users have been logged-in
>>> at the same time (the second customer logged in 3 sec after the first
>>> customer).
>>> The system has more than 1000 customers but this inconvenience happened
>>> for the first time yesterday (?).
>>> It seems that different sessions have been mixed up.
>>> The servlet includes a handle method and uses several other methods
>>> (methods that write and read from a database).
>>> All methods are defined in the same class (that extends WMServlet).
>>> As far as I know servlets must not have instance variables (only local
>>> variables -inside methods- to prevent data corruption and
>>> inconsistencies). However in my servlet I use only one non local
>>> variable (a Logger) and I don't think this is the cause of the problem.
>>> As an attached file I send you a fragment of the before mentioned
>>> servlet.
>>> Can instance variables be defined in the case of a servlet that extends
>>> WMServlet? (I mean is it safe? or its not a good practice without
>>> keeping precautions?)
>>> Do you have any idea what can have caused this inconvenience ?
>>> Is the use of methods inside the servlet thread-safe ?
>>>
>>> Please help!!!
>>> Nikolaos Papadakis
>>> ------------------------------------------------------- This SF.net
>>> email is sponsored by: Splunk Inc. Do you grep through log files for
>>> problems? Stop! Download the new AJAX search engine that makes
>>> searching your log files as easy as surfing the web. DOWNLOAD
>>> SPLUNK!
>>> http://sel.as-us.falkag.net/sel?cmd=k&kid3432&bid#0486&dat1642
>>> _______________________________________________ Webmacro-user
>>> mailing list Web...@li...
>>> https://lists.sourceforge.net/lists/listinfo/webmacro-user
>>
>>
>>
>>
>>
>>
>> -------------------------------------------------------
>> This SF.net email is sponsored by: Splunk Inc. Do you grep through
>> log files
>> for problems? Stop! Download the new AJAX search engine that makes
>> searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
>> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
>> _______________________________________________
>> Webmacro-user mailing list
>> Web...@li...
>> https://lists.sourceforge.net/lists/listinfo/webmacro-user
>>
>>
>------------------------------------------------------------------------
>
>package org.nkpap.visualizer;
>
>import java.io.BufferedOutputStream;
>import java.io.ByteArrayOutputStream;
>import java.io.File;
>import java.io.FileInputStream;
>import java.io.FileNotFoundException;
>import java.io.IOException;
>import java.io.InputStream;
>import java.io.OutputStream;
>import java.io.PrintStream;
>import java.sql.Connection;
>import java.sql.DriverManager;
>import java.sql.PreparedStatement;
>import java.sql.ResultSet;
>import java.sql.SQLException;
>import java.text.DateFormat;
>import java.text.SimpleDateFormat;
>import java.util.Calendar;
>import java.util.Date;
>import java.util.Enumeration;
>import java.util.Hashtable;
>import java.util.LinkedList;
>import java.util.List;
>import java.util.logging.FileHandler;
>import java.util.logging.Logger;
>
>import javax.servlet.ServletException;
>import javax.servlet.http.HttpServletRequest;
>import javax.servlet.http.HttpServletResponse;
>import javax.servlet.http.HttpSession;
>
>import org.netoperis.db.core.Authors;
>import org.netoperis.db.core.Interests;
>import org.netoperis.db.core.Reviews;
>import org.netoperis.db.core.Submissions;
>import org.netoperis.db.core.TopicList;
>import org.netoperis.db.core.Topics;
>import org.netoperis.db.core.Users;
>import org.netoperis.db.core.Votes;
>import org.netoperis.db.mgr.AuthorsManager;
>import org.netoperis.db.mgr.InterestsManager;
>import org.netoperis.db.mgr.ReviewsManager;
>import org.netoperis.db.mgr.SubmissionsManager;
>import org.netoperis.db.mgr.TopicListManager;
>import org.netoperis.db.mgr.TopicsManager;
>import org.netoperis.db.mgr.UsersManager;
>import org.netoperis.db.mgr.VotesManager;
>import org.nkpap.util.EmailBuffer;
>import org.nkpap.util.MyConnectionManager;
>import org.nkpap.util.RandomPasswordGenerator;
>import org.webmacro.PropertyException;
>import org.webmacro.ResourceException;
>import org.webmacro.Template;
>import org.webmacro.servlet.HandlerException;
>import org.webmacro.servlet.WMServlet;
>import org.webmacro.servlet.WebContext;
>import org.webmacro.util.Settings;
>
>import com.oreilly.servlet.MultipartRequest;
>
>/**
> * @author nkpap in Greece 2 ��� 2005
> *
> * (c) 2005 - 2006
> *
> */
>public class Login extends WMServlet {
> public static Settings configProperties = null;
> Logger logger;
>
> protected void start() throws ServletException {
> try {
> configProperties = new Settings();
> System.out.println(" READING CONFIGURATION FILE: ms2.properties");
>
> configProperties.load("ms2.properties");
> System.out
> .println(" CONFIGURATION FILE: ms2.properties LOADED SUCCESSFULLY ");
> FileHandler handler = new FileHandler(configProperties
> .getSetting("LogsPath")
> + "my_log.txt", true);
> logger = Logger.getLogger("org.nkpap.visualizer.LoginUsers");
> logger.addHandler(handler);
> } catch (Exception ex) {
> System.out
> .println("ERROR READING CONFIGURATION FILE: ms2.properties");
> System.out.println(ex);
> }
> }
>
> private Template returnMyTemplate(String template, WebContext context) {
> try {
> context.put("fragment", template);
> return getTemplate("users/index.htm");
> } catch (ResourceException e) {
> System.out
> .println("Oooops!!! a problem occured with the error template!");
> e.printStackTrace();
> return null;
> }
> }
>
> public Template handle(WebContext context) throws HandlerException {
> HttpServletRequest request = context.getRequest();
> HttpSession session = request.getSession();
> context.getResponse().setContentType("text/html; charset=iso-8859-7");
>
> String userID = (String) session.getAttribute("USERID");
>
> if (isActionEqualTo("login", context)) {
> // use has entered credentials
> String login = request.getParameter("login");
> String password = request.getParameter("password");
>
> if (validateUser(login, password, context)) {
> if (session.getAttribute("FIRSTIME") != null)
> return showReviewer(
> (String) session.getAttribute("USERID"), context);
> return returnMyTemplate("users/choices.htm", context);
> }// user doesnt validate
> session.invalidate();
> context.put("fail", "Access denied !!");
> return returnMyTemplate("users/login.htm", context);
> }
> // -----------------------ACTIONS---BEGIN----HERE-------------
> // ACTIONS that do not require user to be logged-in
> if (isActionEqualTo("register", context)) {
> return returnMyTemplate("users/aut_registration.htm", context);
> }
> if (isActionEqualTo("back", context)) {
> return returnMyTemplate("users/login.htm", context);
> }
> if (isActionEqualTo("remind", context)) {
> return returnMyTemplate("users/remind_passwd.htm", context);
> }
> // end of ACTIONS that do not require user to be logged-in
>
> if (isActionEqualTo("logout", context)) {
> session.invalidate();
> logger.info("User " + userID + " logged out");
> return returnMyTemplate("users/login.htm", context);
> }
> // --------------------ACTIONS----END-----HERE---------
> // the default page to be shown...
>
> if (request.getParameter("action") == null && userID != null) {
> if (session.getAttribute("FIRSTIME") != null)
> return showReviewer((String) session.getAttribute("USERID"),
> context);
> return returnMyTemplate("users/choices.htm", context);
> } else {
> return returnMyTemplate("users/login.htm", context);
> }
> }
>
> private Template handleError(Exception e, Connection connection,
> WebContext context) {
> logger.severe("An error occurred:\n" + e.toString());
> if (connection != null) {
> try {
> connection.close();
> } catch (SQLException e1) {
> e1.printStackTrace();
> }
> }
> e.printStackTrace();
> context.put("msg", "An error occurred:\n" + e.toString());
> return returnMyTemplate("users/errore.htm", context);
> }
>
> private Template showReviewer(String userId, WebContext context) {
> MyConnectionManager ConnectionManager = new MyConnectionManager();
> Connection connection = null;
> logger.info("starting method: showReviewer");
> try {
> int usId = Integer.parseInt(userId);
> connection = ConnectionManager.getConnection();
>
> Users user = UsersManager.getManager().loadByKey(usId, connection);
> Topics[] topics = TopicsManager.getManager().loadAll(connection);
>
> connection.close();
> context.put("user", user);
> // context.put("interests", interests);
> context.put("topics", topics);
> } catch (SQLException e) {
> return handleError(e, connection, context);
> }
>
> logger.info("ended method: showReviewer");
> return returnMyTemplate("users/rev_registration.htm", context);
> }
>
> private boolean isActionEqualTo(String actionName, WebContext context) {
> HttpServletRequest request = context.getRequest();
> if (request.getParameter("action") != null
> && request.getParameter("action").equalsIgnoreCase(actionName))
> return true;
> return false;
> }
>
> private Template showJobs(WebContext context) {
> Connection connection = null;
> MyConnectionManager ConnectionManager = new MyConnectionManager();
> HttpServletRequest request = context.getRequest();
> HttpSession session = request.getSession();
> String userId = (String) session.getAttribute("USERID");
> logger.info("starting method: showJobs by user: " + userId);
>
> try {
> connection = ConnectionManager.getConnection();
> Reviews[] reviews = ReviewsManager.getManager().loadByWhere(
> "where USE_ID=\'" + userId + "\'", connection);
> // System.out.println("Papers : " + submissions.length);
>
> for (int i = 0; i < reviews.length; i++) {
>
> Submissions subm = SubmissionsManager.getManager().loadByKey(
> reviews[i].getSubId(), connection);
>
> reviews[i].setForeignTitle(subm.getSubPaperTitle());
> reviews[i].setForeignStatus(subm.getSubStatus());
> }
> connection.close();
> context.put("jobs", reviews);
> logger.info("ended method: showJobs by user: " + userId);
> return returnMyTemplate("users/rev_home.htm", context);
> } catch (SQLException e) {
> return handleError(e, connection, context);
> }
> }
>
> private boolean validateUser(String login, String password,
> WebContext context) {
> MyConnectionManager ConnectionManager = new MyConnectionManager();
> HttpServletRequest request = context.getRequest();
> HttpSession session = request.getSession();
> Connection connection = null;
> logger.info("starting method: validateUser login: " + login
> + " passwd: " + password);
> boolean userOK = false;
> try {
> connection = ConnectionManager.getConnection();
> Users[] foundUsers = UsersManager.getManager().loadByWhere(
> "where USE_EMAIL=\'" + login + "\' AND USE_PASSWORD=\'"
> + password + "\'", connection);
> if (foundUsers.length != 0) {
> userOK = true;
> if (foundUsers[0].getUseActive() == 0) {
> session.setAttribute("FIRSTIME", "TRUE");
> }
> session.setAttribute("USERID", foundUsers[0]
> .getUseId()+"");
> }
> connection.close();
> } catch (SQLException e) {
> handleError(e, connection, context);
> return false;
> }
>
> logger.info("ended method: validateUser: " + userOK + " login: "
> + login + " passwd: " + password);
> return userOK;
> }
>}
>
|
