Re: [WebMacro-user] Session integrity issue?

[Nikolaos P. <nk...@cc...>]

Dear Keats,

thank you for replying.
The code is protected against SQL-injections attacks. Special characters=20
and apostrophes are manipulated properly to avoid potential attackers=20
execute a different sql query.
The classes I use are based on S2J (an O/R mapping tool) with a lot of=20
new additions.

Now concerning the snippet you mentioned, from the servlet developer's=20
prospective, each client is another thread that calls the servlet via=20
the service(), doPost(), or doGet() methods (while only one instance=20
exists).
Every thread will carry its specific userId (through its session given=20
by the servlet container).
Every thread will execute the same =E2=80=9Cwhere USE_EMAIL.....=E2=80=9D=
  snippet but=20
with different parameters (email address and password in this case).
So I think there is no need the loadByWhere() method to be synchronized.
The only synchronized mechanism, however, is used in the part of code=20
that is responsible for inserting a new column into the db (because I am=20
not using the autoincrement feature of the specific RDBMS), but that's=20
not pertinent here.

Please correct me if I've gotten it wrong.



Best Regards,
Nikos Papadakis


Keats Kirsch wrote:
> Nikolaos,
>
> I don't know S2J, but I assume it's an O/R mapping tool.  I would=20
> focus my attention on the following line of code:
>
>            Users[] foundUsers =3D UsersManager.getManager().loadByWhere=
(
>                    "where USE_EMAIL=3D\'" + login + "\' AND=20
> USE_PASSWORD=3D\'"
>                            + password + "\'", connection);
>
> Is this invoking customized code?  Is loadByWhere() synchronized, or=20
> is there some other synchronization mechanism here?
>
> (Btw, unless the S2J protects against it, this code could be=20
> vulnerable to a SQL-injection attack.  Make sure you check the userid=20
> and password for apostrophes or other special characters that might=20
> 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=20
>> 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=20
>> 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 =E2=80=9Cobjectize=E2=80=9D=
 tables and=20
>> relations in the db.
>>
>>
>>
>>
>>
>> Keats Kirsch wrote:
>>
>>> It's not possible to tell from the code snippet what is going=20
>>> wrong.  The concurrency issue that Lane is referring to is fairly=20
>>> obscure (only affecting the #include directive with dynamic template=20
>>> names if I recall correctly) and not relevant here.
>>>
>>> The code as shown looks OK, but we can't see how the USERID is=20
>>> stored in the session, or how the user data is stored and retrieved=20
>>> based on this USERID.  The bug could be in the data access layer or=20
>>> the session management subsystem (which depends on your Servlet=20
>>> container), or in some caching mechanism.  It is unlikely, but not=20
>>> impossible that this could be a WebMacro bug.  If I had to guess, I=20
>>> would suspect that the code which actually retrieves the user data=20
>>> is not properly synchronized.
>>>
>>> Send us a bit more information about your application, and maybe we=20
>>> 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=20
>>>> possibly in prior releases. If you look at the mail thread dating=20
>>>> back for 45 days, you will definitely see it.
>>>>
>>>> My concern is that the webmacro instance, shared, and the context,=20
>>>> not shared, is improperly orchestrated for concurrency. The=20
>>>> webmacro instance is local to WMServlet. You might consider=20
>>>> synchronizing access to this variable as a trial fix in your local=20
>>>> calls below. This will introduce some serialization of web=20
>>>> processing but it might help to verify the solution.
>>>>
>>>> One thing you might consider trying is to use a different cache=20
>>>> handler. A few years ago, I wrote one and it is a part of the=20
>>>> 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=20
>>>> that he
>>>> managed to =E2=80=9Csee=E2=80=9D the account of another customer whe=
n he logged-in=20
>>>> using
>>>> his own credentials (he send me a screen shot of the =E2=80=9Cview=E2=
=80=9D).
>>>> 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 firs=
t
>>>> customer).
>>>> The system has more than 1000 customers but this inconvenience=20
>>>> 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 loc=
al
>>>> 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=20
>>>> problem.
>>>> As an attached file I send you a fragment of the before mentioned=20
>>>> servlet.
>>>> Can instance variables be defined in the case of a servlet that=20
>>>> 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=20
>>>> email is sponsored by: Splunk Inc. Do you grep through log files=20
>>>> for problems? Stop! Download the new AJAX search engine that makes=20
>>>> searching your log files as easy as surfing the web. DOWNLOAD=20
>>>> SPLUNK!=20
>>>> http://sel.as-us.falkag.net/sel?cmd=3Dk&kid=103432&bid#0486&dat=1216=
42=20
>>>> _______________________________________________ Webmacro-user=20
>>>> mailing list Web...@li...=20
>>>> https://lists.sourceforge.net/lists/listinfo/webmacro-user=20
>>>
>>>
>>>
>>>
>>>
>>>
>>> -------------------------------------------------------
>>> This SF.net email is sponsored by: Splunk Inc. Do you grep through=20
>>> log files
>>> for problems?  Stop!  Download the new AJAX search engine that makes
>>> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUN=
K!
>>> http://sel.as-us.falkag.net/sel?cmd=3Dlnk&kid=3D103432&bid=3D230486&d=
at=3D121642=20
>>>
>>> _______________________________________________
>>> 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 =EF=BF=BD=EF=BF=BD=EF=BF=BD 2005
>> * * (c) 2005 - 2006
>> * */
>> public class Login extends WMServlet {
>>     public static Settings configProperties =3D null;
>>     Logger logger;
>>
>>     protected void start() throws ServletException {
>>         try {
>>             configProperties =3D new Settings();
>>             System.out.println(" READING CONFIGURATION FILE:=20
>> ms2.properties");
>>
>>             configProperties.load("ms2.properties");
>>             System.out
>>                     .println("  CONFIGURATION FILE: ms2.properties=20
>> LOADED SUCCESSFULLY ");
>>             FileHandler handler =3D new FileHandler(configProperties
>>                     .getSetting("LogsPath")
>>                     + "my_log.txt", true);
>>             logger =3D=20
>> Logger.getLogger("org.nkpap.visualizer.LoginUsers");
>>             logger.addHandler(handler);
>>         } catch (Exception ex) {
>>             System.out
>>                     .println("ERROR READING CONFIGURATION FILE:=20
>> ms2.properties");
>>             System.out.println(ex);
>>         }
>>     }
>>
>>     private Template returnMyTemplate(String template, WebContext=20
>> context) {
>>         try {
>>             context.put("fragment", template);
>>             return getTemplate("users/index.htm");
>>         } catch (ResourceException e) {
>>             System.out
>>                     .println("Oooops!!!  a problem occured with the=20
>> error template!");
>>             e.printStackTrace();
>>             return null;
>>         }
>>     }
>>
>>     public Template handle(WebContext context) throws HandlerException=
 {
>>         HttpServletRequest request =3D context.getRequest();
>>         HttpSession session =3D request.getSession();
>>         context.getResponse().setContentType("text/html;=20
>> charset=3Diso-8859-7");
>>
>>         String userID =3D (String) session.getAttribute("USERID");
>>
>>         if (isActionEqualTo("login", context)) {
>>             // use has entered credentials
>>             String login =3D request.getParameter("login");
>>             String password =3D request.getParameter("password");
>>
>>             if (validateUser(login, password, context)) {
>>                 if (session.getAttribute("FIRSTIME") !=3D null)
>>                     return showReviewer(
>>                             (String) session.getAttribute("USERID"),=20
>> 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",=20
>> 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);
>>         }      =20
>>         // --------------------ACTIONS----END-----HERE---------
>>         // the default page to be shown...
>>
>>         if (request.getParameter("action") =3D=3D null && userID !=3D =
null) {
>>             if (session.getAttribute("FIRSTIME") !=3D null)
>>                 return showReviewer((String)=20
>> 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 !=3D 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 =3D new=20
>> MyConnectionManager();
>>         Connection connection =3D null;
>>         logger.info("starting method: showReviewer");
>>         try {
>>             int usId =3D Integer.parseInt(userId);
>>             connection =3D ConnectionManager.getConnection();
>>
>>             Users user =3D UsersManager.getManager().loadByKey(usId,=20
>> connection);
>>             Topics[] topics =3D=20
>> 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=20
>> context) {
>>         HttpServletRequest request =3D context.getRequest();
>>         if (request.getParameter("action") !=3D null
>>                 &&=20
>> request.getParameter("action").equalsIgnoreCase(actionName))
>>             return true;
>>         return false;
>>     }
>>
>>     private Template showJobs(WebContext context) {
>>         Connection connection =3D null;
>>         MyConnectionManager ConnectionManager =3D new=20
>> MyConnectionManager();
>>         HttpServletRequest request =3D context.getRequest();
>>         HttpSession session =3D request.getSession();
>>         String userId =3D (String) session.getAttribute("USERID");
>>         logger.info("starting method: showJobs by user: " + userId);
>>
>>         try {
>>             connection =3D ConnectionManager.getConnection();
>>             Reviews[] reviews =3D ReviewsManager.getManager().loadByWh=
ere(
>>                     "where USE_ID=3D\'" + userId + "\'", connection);
>>             // System.out.println("Papers : " + submissions.length);
>>
>>             for (int i =3D 0; i < reviews.length; i++) {
>>
>>                 Submissions subm =3D=20
>> 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 =3D new=20
>> MyConnectionManager();
>>         HttpServletRequest request =3D context.getRequest();
>>         HttpSession session =3D request.getSession();
>>         Connection connection =3D null;
>>         logger.info("starting method: validateUser login: " + login
>>                 + " passwd: " + password);
>>         boolean userOK =3D false;
>>         try {
>>             connection =3D ConnectionManager.getConnection();
>>             Users[] foundUsers =3D UsersManager.getManager().loadByWhe=
re(
>>                     "where USE_EMAIL=3D\'" + login + "\' AND=20
>> USE_PASSWORD=3D\'"
>>                             + password + "\'", connection);
>>             if (foundUsers.length !=3D 0) {
>>                 userOK =3D true;
>>                 if (foundUsers[0].getUseActive() =3D=3D 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;
>>     }
>> }
>>
>
>
>
>
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log=20
> 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=3Dlnk&kid=3D103432&bid=3D230486&dat=
=3D121642
> _______________________________________________
> Webmacro-user mailing list
> Web...@li...
> https://lists.sourceforge.net/lists/listinfo/webmacro-user
>
>