modarwish
modarwish

Reputation: 72

How can I secure a JSP page after adding it to my hosting and making it live?

I have a very basic login JSP that passes the variables to the servlet and checks from a MySQL DB if the username and password are available. Is this secure enough to use on a website, or does it need more security? If so, how to make it more secure?

This is the servlet:

import java.io.*;
import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.*;
import java.sql.*;

/**
 * Servlet implementation class loginServlet
*/
@WebServlet("/loginServlet")
public class loginServlet extends HttpServlet {

private static final long serialVersionUID = 1L;

/**
 * @seeHttpServlet#HttpServlet()
 */
public loginServlet() {
    super();
    // TODOAuto-generated constructor stub
}

/**
 * @seeHttpServlet#doGet(HttpServletRequest request, HttpServletResponse
 * response)
 */
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
    // TODOAuto-generated method stub
}

/**
 * @seeHttpServlet#doPost(HttpServletRequest request, HttpServletResponse
 * response)
 */
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
    HttpSession session = request.getSession();
    String email = request.getParameter("email");
    String pwd = request.getParameter("pass");
    try {
        Class.forName("com.mysql.jdbc.Driver");
        Connection con =
                DriverManager.getConnection("jdbc:mysql://localhost:3306/logindb",
                "root", "password");
        Statement st = con.createStatement();
        ResultSet rs;
        rs = st.executeQuery("select fname, lname, email from userAccount where Email='"
                + email + "' and password='" + pwd + "'");
        if (rs.next()) {
            session.setAttribute("email", email);
            session.setAttribute("Fullname", rs.getString(1) + " " + rs.getString(2));
            response.sendRedirect("success.jsp");
        } else {

            response.sendRedirect("fail.jsp");
        }
    } catch (Exception ssd) {
        System.out.println(ssd.getMessage());
    }
}
}

Upvotes: 4

Views: 1570

Answers (5)

Masudul
Masudul

Reputation: 21981

Is this secure enough to use on a website, or does it need more security? If so, how to make it more secure?

No. this is not enough secure. You need to use form-based authentication, store password as hash and restrict direct resource invocation. For that, I prefer Spring Security. Following benefits you will get from Spring Security.

  1. Basic Spring Security configuration
  2. OpenID integration
  3. Access Control List (ACL)
  4. JDBC-based configuration
  5. Remember-me services
  6. LDAP-based authentication
  7. Single Sign-on services
  8. JSF and GWT integration
  9. and many more

Upvotes: 2

Vivek Viswanathan
Vivek Viswanathan

Reputation: 1963

The above is insecure for the following reasons,

  1. SQL Injection: If you see the below code, you are directly appending the user input to the SQL query. So lets say a user provided the email as "';drop table userAccount;". This would drop the table.

    rs = st.executeQuery("select fname, lname, email from userAccount where Email='"+ email + "' and password='"+ pwd + "'");

  2. Never show stack trace to user: If the code above throws any exception inside the try block, you are catching it and printing in console. But there is no response being sent. You can redirect the user to fail.jsp in that case as well.

  3. Use Capcha or any token mechanism to avoid automated submissions.

Upvotes: 1

aalku
aalku

Reputation: 2878

You definitely should not store crear passwords so if you are hacked the hacker does not get the passwords. You should digest them with a non-reversible algorithm (SHA-1 recommended) with salt. Salt is a protection against rainbow tables.

Upvotes: 0

DavidC
DavidC

Reputation: 1862

It looks like your password is not hashed in the database. So before storing the password in the database call eg sha256 (https://stackoverflow.com/a/5531479/514463) and then when you are looking up the password in your above servlet do it again.

st.executeQuery("select fname, lname, email from userAccount where Email='"+ email + "' and password='"+ sha256(pwd) + "'"); 

Furthermore - you are not using bind variables in your sql so your code is open to sql injection which means someone could pass in as a password somehtlin like

  "password; delete from users;"

and after your sql is executed the users table could all be deleted. Always use prepared statements

 dbConnection.prepareStatement("select fname, lname, email from userAccount where Email=? and password=?" );

passing in your username and sha256(password)

Upvotes: 0

JB Nizet
JB Nizet

Reputation: 692121

There are several security issues, and programming problems, with this code:

  • unless the application is served over HTTPS, the password passes in clear text over the network
  • passwords should not be stored in clear in a database. They should be salted using a random salt, and then hashed using a slow cryptographic algorithm like bcrypt. To check the password, you should salt and hash the input from the user, and compare the result with the salted and hashed password stored in the database
  • your code doesn't use prepared statements, opening itself to SQL injection attacks
  • your code doesn't use prepared stataments, which will make it fail, for example, as soon as there is a single quote inside the email or the password.
  • you shouldn't catch Exception. Only catch exceptions that you can handle, and that are supposed to happen. For unexpected exceptions, displaying a generic error page is fine. For expected exceptions, you should handle them. Your catch block logs something in the server console, and leaves the user with a blank page.

Upvotes: 4

Related Questions