Что вопрос безопасности в этом коде?


Меня спросили, как этот код имеет риск для безопасности. У кого-нибудь есть идеи, что это такое? Я новичок по теме Безопасность и не знаете, что искать.

String DBdriver = "com.ora.jdbc.Driver";
String DataURL = "jdbc:db://localhost:5112/users";
String loginName = "stackoverflow";
String passwd = "codeReview";
Class.forName(DBdriver);
Connection conn = DriverManager.getConnection(DataURL, loginName, passwd);
String Username = request.getParameter("USER");
String Password = request.getParameter("PASSWORD");
int iUserID = -1;
String loginUser = "";
String sel = "SELECT UserID, Username FROM USERS WHERE Username = '" 
    +Username + "' AND Password = '" + Password + "'";
Statement selectStatement = conn.createStatement ();
ResultSet result = selectStatement.executeQuery(sel);
if (result.next()) {
       iUserID = result.getInt(1);
       loginUser = result.getString(2);
}
PrintWriter wr = response.getWriter ();
if (iUserID >= 0) {
       wr.println ("you logged in: " + loginUser);
} else {
       wr.println ("you cannot login in !Access Denied!")
}


514
2
задан 9 декабря 2011 в 02:12 Источник Поделиться
Комментарии
3 ответа

С верхней части моей головы:


  1. Эти параметры, кажется, не быть экранированы, прежде чем сшитые в запросе (открытия атак SQL-инъекций)

  2. База данных будет держать пароли в открытом виде, а не в виде соленых хэшей (что особенно плохое сочетание с предыдущим пунктом)

Ты действительно хочешь


  • побег имя пользователя и пароль, прежде чем бросить их в Выбрать заявление

  • сохранить пароль хранится как значение хэша как минимум (потом ты захочешь проверить вход пользователей путем хэширования пароля вы получаете от них и сравнивать, что к чему у вас хранится). В идеале, вы бы соли им тоже.

Это не больно, чтобы принять логин и пароль в посте параметр либо (просто чтобы сократить число журналов имя пользователя/пароль комбо в итоге).

9
ответ дан 9 декабря 2011 в 03:12 Источник Поделиться

Некоторые другие заметки:


  1. Не смешивать стили именования:

    String DataURL = "jdbc:db://localhost:5112/users";
    String loginName = "stackoverflow"

    Имена переменных должны быть верблюжьего (со строчной первой буквой).


  2. Вместо того, чтобы проверить, что iUserID это -1 или не проверить результат результат.следующий() и установить логическое значение флага:

    final boolean hasUser = result.next();
    if (hasUser) {
    iUserID = result.getInt(1);
    loginUser = result.getString(2);
    } else {
    wr.println ("you cannot login in !Access Denied!");
    }

    Или даже лучше:

    final boolean hasUser = result.next();
    if (!hasUser) {
    wr.println ("you cannot login in !Access Denied!");
    return; // etc.
    }
    final int userID = result.getInt(1);
    final String loginUser = result.getString(2);

    Он удаляет магических строк из кода и делает его более удобным для чтения.

    Обратите внимание, что я тоже перешел с объявления переменных. (Эффективная Java, второе издание, пункт 45: минимизировать область видимости локальных переменных имеет хороший обзор на эту тему. (Google для "минимизации видимости локальных переменных", это в Google книгах тоже.))


  3. Вместо результатов.получить*(инт значение columnindex) методы использования результатов.получить*(строка columnLabel) из них:

    userID = result.getInt("UserID");
    loginUser = result.getString("Username);

    Он также удаляет некоторые магические цифры из кода и делает его более удобным для чтения.


  4. Это:

    PrintWriter wr = response.getWriter ();

    должно быть:

    PrintWriter wr = response.getWriter();

    (Из здесь):


    Обратите внимание, что пробел не должен быть использован между именем метода и
    его открывающей скобкой. Это помогает отличить ключевые слова от
    вызовы метода.

  5. Я бы использовал более длинные имена переменных для удобочитаемости:

    PrintWriter writer = response.getWriter();

3
ответ дан 10 декабря 2011 в 02:12 Источник Поделиться

Существуют три проблемы, от наименьшего до наибольшего.


  1. Данные подключение к базе данных в коде. Должны быть в
    отдельный конфигурационный файл, так что он может быть изменен и
    разработчику не нужно знать пароль производства. В зависимости от
    после установки (один Дэв делает, например, разработчиков и DBA), это может
    не будет большой проблемой.

  2. Уязвимость SQL-инъекции, имя пользователя '; удаление пользователей таблице; -- может просто
    испортить день Йор.

  3. Вы храните пароль пользователя в открытом виде. Вы никогда не должны хранить пользователей
    пароль, вы должны хранить хэш пароля пользователя. Пользователи рециркулирует
    пароли. Если вы храните пароль, и ваша система будет взломана (или у вас есть
    рассержанный сотрудник), которые могут быть использованы для доступа к другим учетным записям. Такой
    казенник может открыть ваш работодатель к большой ответственности...
    Потому что это влияет несколько систем, это наиболее серьезная.

2
ответ дан 19 марта 2012 в 12:03 Источник Поделиться