Как исправить проблему с SQL-инъекцией усечения таблицы

Ниже приведена строка кода, в которой я усекаю записи таблицы. Значение таблицы поступает из внешнего интерфейса. В моем сканировании Veracode отображается SQL-инъекция. Как я могу этого избежать? Я не могу создать хранимую процедуру, поскольку строка подключения является динамической, и мне нужно обрезать эту таблицу. Есть ли другой подход?

SqlCommand cmd = connection.CreateCommand();
cmd.Transaction = transaction;
cmd.CommandText = "TRUNCATE TABLE " + tablename;
cmd.ExecuteNonQuery();

person Sarvesh Gupta    schedule 15.05.2020    source источник
comment
можно пометить как ложноположительный. Но я согласен, это плохой ход. Почему вы не можете создать процедуру(ы)?   -  person T.S.    schedule 15.05.2020
comment
сколько таблиц может быть отправлено на этот код? Я бы сказал, поместите список возможных таблиц в массив или хэш-набор, и ваш код должен убедиться, что переданное значение имени таблицы находится там. Затем используйте значение из массива вместо переданного значения.   -  person JoelFan    schedule 15.05.2020
comment
Это не совсем ложное срабатывание. Пока что. Сначала вам нужно убедиться, что вы правильно экранируете имя.   -  person madreflection    schedule 15.05.2020
comment
или используйте перечисляемый тип или константную строку для имени таблицы вместо обычной строки   -  person JoelFan    schedule 15.05.2020
comment
string sqlStatement = string.Format(TRUNCATE TABLE [{0}], tableName);   -  person Samim Hussain    schedule 15.05.2020
comment
@TS, как я уже сказал, строка подключения является динамической, и мне нужно обрезать таблицу, мы формируем значение строки подключения из значения БД, которое поступает из внешнего интерфейса, и выполняем этот оператор усечения в этом контексте строки подключения, как поможет sp и где мы поставили этот sp   -  person Sarvesh Gupta    schedule 15.05.2020
comment
просто примечание... я долгое время программировал с использованием sql, и у меня никогда не было необходимости в коде приложения (особенно многоуровневого!) для усечения таблицы, поэтому вы можете пересмотреть свой дизайн. Если вы не пишете интерфейс для инструмента администрирования базы данных, я думаю, что есть лучший способ   -  person JoelFan    schedule 15.05.2020
comment
@JoelFan Я понимаю, Джо, это не разработано мной, и это очень старый код, я только что получил задание исправить эти проблемы, поэтому просто хотел получить эффективную помощь, если мы сможем что-то сделать, не переделывая всю архитектуру.   -  person Sarvesh Gupta    schedule 15.05.2020
comment
@SarveshGupta Каждая БД должна будет содержать этот SP, где вы передаете таблицу и выполняете динамический код. Неважно, откуда вы получаете связь.   -  person T.S.    schedule 16.05.2020


Ответы (3)


Вам нужен динамический sql:

string sql = @"
    DECLARE @SQL nvarchar(150); 
    SELECT @SQL = 'truncate table ' + quotename(table_name) + ';'
        FROM information_schema.tables 
        WHERE table_name = @table;
    EXEC(@SQL);";

using (var connection = new SqlConnection("connection string here"))
using (var cmd = new SqlCommand(sql, connection))
{
    cmd.Transaction = transaction;
    cmd.Parameters.Add("@table", SqlDbType.NVarChar, 128).Value = tablename;
    connection.Open();
    cmd.ExecuteNonQuery();
}

Это один из немногих случаев, когда динамический SQL делает вещи более безопасными, а не менее. Еще лучше, если вы также поддерживаете специальную таблицу в этой базе данных, в которой перечислены другие таблицы, которые пользователям разрешено усекать, и используйте ее, а не information_schema для проверки имени. Идея позволить пользователям просто обрезать что-нибудь немного пугает.

person Joel Coehoorn    schedule 15.05.2020
comment
это не создаст ошибку инъекции sql ?? поскольку имя таблицы исходит из внешнего интерфейса - person Sarvesh Gupta; 15.05.2020
comment
@SarveshGupta Нет, так как имя этой таблицы параметризовано здесь. - person Rahul; 15.05.2020
comment
@SarveshGupta Это циклически повторяет ввод через базу данных безопасным способом, чтобы убедиться, что у вас есть действительное имя таблицы. Остается вопрос, почему вы разрешаете выбирать и полностью усекать любое имя таблицы. - person Joel Coehoorn; 15.05.2020
comment
@SarveshGupta Добавлен код для контекста. Теперь вы можете видеть, где таблица передается как безопасное значение параметра. - person Joel Coehoorn; 15.05.2020
comment
Параметризация @Rahul не делает его волшебным образом безопасным. Это другие шаги - person Martin Smith; 15.05.2020
comment
Никакая параметризация не будет иметь значения для динамического SQL, если она не экранирована должным образом. Это то, что QUOTENAME там делает. Все зависит от этого. - person madreflection; 15.05.2020
comment
@madreflection Также я извлек имя таблицы из базы данных. Если пользователь вводит что-то, что не является реальной таблицей, цитировать нечего. Эти два вместе означают, что вы, по крайней мере, получаете один действительный оператор sql, который делает именно то, что вы ожидаете. Однако на моем месте у меня также была бы специальная таблица в этой базе данных, в которой перечислены определенные наборы других таблиц, которые пользователям разрешено усекать. - person Joel Coehoorn; 15.05.2020

Параметризованный или нет, в этом случае вы можете сделать его лишь немного более защищенным. Никогда не был полностью обеспечен. Для этого вам нужно

создайте таблицу TruncMapping в БД, где вы будете хранить
id guid
statement varchar(300)
ваши данные будут выглядеть так
SOME-GUID-XXX-YYY, 'TRUNCATE TABLE TBL1'

  • В интерфейсе используйте listbox или combobox с текстом/значением, например "Customer Data"/"SOME-GUID-XXX-YYY"
  • В вашем коде используйте ExecuteScalar для выполнения Select statement from TruncMapping where id = @1 , где id будет параметризованным GUID из комбинированного значения.
  • Выполните команду усечения, используя ExecuteNonQuery, как вы это делаете сейчас, но с извлеченной строкой из предыдущего вызова.

Ваш сканер, скорее всего, задохнется. Если он все еще думает, что код небезопасен, вы можете смело указывать это как ложное срабатывание, потому что то, что вы выполняете, исходит из вашей защищенной БД. У потенциального злоумышленника нет возможности саботировать ваши «неизменяемые таблицы», потому что они не указаны в TruncMapping таблицах.

Вы только что создали многоуровневую защиту от SQL-инъекций.

person T.S.    schedule 15.05.2020

вот один из способов скрыть его от инструментов сканирования

private const string _sql = "VFJVTkNBVEUgVEFCTEU=";
. . . . 

var temp = new { t = tablename };
cmd.CommandText = 
    Encoding.ASCII.GetString(Convert.FromBase64String(_sql)) + temp.t.PadLeft(temp.t.Length + 1);

безопасность через неизвестность

person T.S.    schedule 15.05.2020
comment
Разве это не использование неясности для преодоления абсолютно законной проблемы безопасности, обнаруженной анализом кода? - person Martin Smith; 15.05.2020
comment
@MartinSmith, вы параметризуете это или нет, если вы получаете что-то из текстового поля и объединяете с Truncate Table, это не защищено. Что более безопасно, так это то, что элемент управления представляет собой комбо или список, если он проверяется по списку таблиц и т. Д. Здесь мы имеем дело строго с поражением инструмента сканирования. - person T.S.; 15.05.2020
comment
Зачем вам пытаться победить инструмент сканирования, который вы запускаете сами? Не запускайте статический анализ кода, если считаете, что это нецелесообразно, или если вы оценили его в контексте и сочли его безопасным, используйте любой метод, который используется для пометки как безопасный. Делать код сильно запутанным, чтобы никто не видел, что он делает, — не очень хорошее решение. - person Martin Smith; 15.05.2020
comment
@MartinSmith Давайте поговорим о текущей ситуации. Единственный более или менее безопасный способ сделать это — получить список с каким-то идентификатором во внешнем интерфейсе, передать его во внутренний интерфейс и сверить, скажем, с таблицей. Не таблица информационной схемы, а пользовательская таблица, которая содержит этот идентификатор и фактический оператор tuncate. И желательно, чтобы этот Id был каким-нибудь Guid. Теперь инструменты сканирования будут жаловаться на все эти вещи, если вам это нужно, и точка. Эти инструменты жалуются, что ваша переменная называется password или pwd. Так что да, мы называем их p_zz4rd или как-то так, просто чтобы инструмент перестал ныть - person T.S.; 15.05.2020