Почему этот код уязвим для атак переполнения буфера?

148
int func(char* str)
{
   char buffer[100];
   unsigned short len = strlen(str);

   if(len >= 100)
   {
        return (-1);
   }

   strncpy(buffer,str,strlen(str));
   return 0;
}

Этот код уязвим для атаки переполнения буфера, и я пытаюсь выяснить, почему. Я думаю, что это связано с lenтем, чтобы быть объявленным shortвместо int, но я не совсем уверен.

Любые идеи?

Джейсон
источник
3
Есть несколько проблем с этим кодом. Напомним, что строки C заканчиваются нулем.
Дмитрий Чубаров
4
@DmitriChubarov, не нулевое завершение строки будет проблемой, только если строка используется после вызова strncpy. В этом случае это не так.
Р Саху
43
Проблемы в этом коде проистекают непосредственно из того факта, что strlenвычисляется, используется для проверки достоверности, а затем он снова абсурдно вычисляется - это СУХОЙ сбой. Если бы второе strlen(str)было заменено на len, не было бы возможности переполнения буфера, независимо от типа len. Ответы не касаются этого вопроса, им просто удается избежать этого.
Джим Балтер
3
@CiaPan: если передать ему строку, не заканчивающуюся нулем, strlen покажет неопределенное поведение.
Кайзерлуди
3
@JimBalter Нет, я думаю, что оставлю их там. Возможно, кто-то еще будет иметь такое же глупое заблуждение и учиться на нем. Не стесняйтесь помечать их, если они вас раздражают, кто-то может прийти и удалить их.
Асад Саидуддин

Ответы:

192

На большинстве компиляторов максимальное значение unsigned shortравно 65535.

Любое значение выше этого оборачивается, поэтому 65536 становится 0, а 65600 становится 65.

Это означает, что длинные строки правильной длины (например, 65600) пройдут проверку и переполнят буфер.


Используйте size_tдля хранения результата strlen(), а не для unsigned shortсравнения lenс выражением, которое непосредственно кодирует размер buffer. Так, например:

char buffer[100];
size_t len = strlen(str);
if (len >= sizeof(buffer) / sizeof(buffer[0]))  return -1;
memcpy(buffer, str, len + 1);
orlp
источник
2
@PatrickRoberts Теоретически да. Но вы должны иметь в виду, что 10% кода отвечает за 90% времени выполнения, поэтому вы не должны снижать производительность до обеспечения безопасности. И имейте в виду, что со временем код меняется, что может внезапно означать, что предыдущая проверка прошла.
orlp
3
Чтобы предотвратить переполнение буфера, просто используйте lenв качестве третьего аргумента strncpy. Использование strlen снова глупо в любом случае.
Джим Балтер
16
/ sizeof(buffer[0])- обратите внимание, что sizeof(char)в C всегда 1 (даже если char содержит gazillion битов), так что это излишне, когда нет возможности использовать другой тип данных. Тем не менее ... благодарность за полный ответ (и спасибо за отзывчивость к комментариям).
Джим Балтер
4
@ rr-: char[]и char*это не одно и то же. Есть много ситуаций, в которых char[]неявно преобразуется в char*. Например, char[]точно так же, как char*при использовании в качестве типа для аргументов функции. Однако конвертация не происходит за sizeof().
Дитрих Эпп
4
@Controll Потому что, если вы измените размер bufferв какой-то момент, выражение автоматически обновится. Это очень важно для безопасности, потому что объявление bufferможет быть довольно далеко от проверки в реальном коде. Таким образом, легко изменить размер буфера, но не забудьте обновить его в каждом месте, где используется размер.
orlp
28

Проблема здесь:

strncpy(buffer,str,strlen(str));
                   ^^^^^^^^^^^

Если строка превышает длину целевого буфера, strncpy все равно скопирует ее. Вы основываете количество символов строки в качестве числа для копирования вместо размера буфера. Правильный способ сделать это заключается в следующем:

strncpy(buffer,str, sizeof(buff) - 1);
buffer[sizeof(buff) - 1] = '\0';

Это ограничивает объем копируемых данных фактическим размером буфера минус один для нулевого завершающего символа. Затем мы устанавливаем последний байт в буфере нулевым символом в качестве дополнительной защиты. Причина этого в том, что strncpy будет копировать до n байтов, включая завершающий ноль, если strlen (str) <len - 1. Если нет, то ноль не копируется, и у вас есть сценарий сбоя, потому что теперь ваш буфер имеет неопределенный строка.

Надеюсь это поможет.

РЕДАКТИРОВАТЬ: После дальнейшего изучения и ввода от других, возможное кодирование для функции следующим образом:

int func (char *str)
  {
    char buffer[100];
    unsigned short size = sizeof(buffer);
    unsigned short len = strlen(str);

    if (len > size - 1) return(-1);
    memcpy(buffer, str, len + 1);
    buffer[size - 1] = '\0';
    return(0);
  }

Поскольку мы уже знаем длину строки, мы можем использовать memcpy, чтобы скопировать строку из местоположения, на которое ссылается str, в буфер. Обратите внимание, что на странице руководства для strlen (3) (в системе FreeBSD 9.3) указано следующее:

 The strlen() function returns the number of characters that precede the
 terminating NUL character.  The strnlen() function returns either the
 same result as strlen() or maxlen, whichever is smaller.

Который я интерпретирую как то, что длина строки не включает ноль. Вот почему я копирую len + 1 байт, чтобы включить ноль, и тест проверяет, чтобы убедиться, что длина <размер буфера - 2. Минус один, потому что буфер начинается с позиции 0, и минус другой, чтобы убедиться, что есть место для нуля.

РЕДАКТИРОВАТЬ: Оказывается, размер чего-то начинается с 1, а доступ начинается с 0, так что -2 раньше был неправильным, потому что он вернул бы ошибку для всего> 98 байтов, но это должно быть> 99 байтов.

РЕДАКТИРОВАТЬ: Хотя ответ о беззнаковых коротких, как правило, является правильным, так как максимальная длина, которая может быть представлена, составляет 65 535 символов, это на самом деле не имеет значения, потому что если строка длиннее, значение будет перенесено. Это все равно, что взять 75 231 (что 0x000125DF) и замаскировать верхние 16 бит, что даст вам 9695 (0x000025DF). Единственная проблема, с которой я сталкиваюсь, это первые 100 символов после 65 535, поскольку проверка длины разрешит копирование, но во всех случаях будет копироваться только до первых 100 символов строки, и нулевое завершение строки . Таким образом, даже в случае проблемы с переносом, буфер все равно не будет переполнен.

Это может или не может само по себе представлять угрозу безопасности в зависимости от содержимого строки и того, для чего вы ее используете. Если это просто прямой текст, который читается человеком, то обычно проблем нет. Вы просто получаете усеченную строку. Однако, если это что-то вроде URL или даже последовательности команд SQL, у вас могут возникнуть проблемы.

Даниэль Руди
источник
2
Правда, но это выходит за рамки вопроса. Код ясно показывает функцию, переданную указателю на символ. Вне сферы действия функции нам все равно.
Даниэль Руди
«буфер, в котором хранится str» - это не переполнение буфера , которое здесь является проблемой. И у каждого ответа есть эта «проблема», которая неизбежна, учитывая сигнатуру func... и любую другую когда-либо написанную функцию C, которая принимает в качестве аргументов строки с NUL-концевыми символами. Воспоминание о том, что вход не завершен NUL, совершенно не имеет смысла.
Джим Балтер
«это выходит за рамки вопроса», что, к сожалению, не поддается пониманию некоторых людей.
Джим Балтер
«Проблема здесь» - вы правы, но вам все еще не хватает ключевой проблемы, которая заключается в том, что test ( len >= 100) был выполнен для одного значения, но длина копии была присвоена другое значение ... это является нарушением принципа СУХОЙ. Простой вызов strncpy(buffer, str, len)исключает возможность переполнения буфера и выполняет меньше работы, чем strncpy(buffer,str,sizeof(buffer) - 1)... хотя здесь это просто более медленный эквивалент memcpy(buffer, str, len).
Джим Балтер
@JimBalter Это выходит за рамки вопроса, но я отвлекся. Я понимаю, что значения, используемые тестом, и значения, используемые в strncpy, являются двумя разными. Тем не менее, обычная практика кодирования гласит, что предел копирования должен быть sizeof (buffer) - 1, поэтому не имеет значения, какой длины str соответствует копии. strncpy прекратит копирование байтов, когда он достигнет нуля или скопирует n байтов. Следующая строка гарантирует, что последний байт в буфере является нулевым символом. Код безопасен, я придерживаюсь своего предыдущего заявления.
Даниэль Руди
11

Даже если вы используете strncpy, длина обрезки по-прежнему зависит от переданного указателя строки. Вы не представляете, какой длины эта строка (то есть расположение нулевого терминатора относительно указателя). Таким образом, strlenодин только вызов открывает вам уязвимость. Если вы хотите быть более безопасным, используйте strnlen(str, 100).

Полный исправленный код будет:

int func(char *str) {
   char buffer[100];
   unsigned short len = strnlen(str, 100); // sizeof buffer

   if (len >= 100) {
     return -1;
   }

   strcpy(buffer, str); // this is safe since null terminator is less than 100th index
   return 0;
}
Патрик Робертс
источник
@ user3386109 Не будет ли strlenтогда доступ за концом буфера?
Патрик Робертс
2
@ user3386109 то, на что вы указываете, делает ответ orlp таким же недействительным, как и мой. Я не понимаю, почему strnlenне решает проблему, если то, что предлагает orlp, в любом случае, верно.
Патрик Робертс
1
«Я не думаю, что strnlen решает что-то здесь» - конечно, это делает; это предотвращает переполнение buffer. «поскольку str может указывать на буфер из 2 байтов, ни один из которых не является NUL». - это не имеет значения, как это верно для любой реализации func. Вопрос здесь о переполнении буфера, а не UB, потому что ввод не завершен NUL.
Джим Балтер
1
«Второй параметр, передаваемый в strnlen, должен быть размером объекта, на который указывает первый параметр, иначе strnlen бесполезен», - это полная и полная чушь. Если вторым аргументом для strnlen является длина входной строки, то strnlen эквивалентен strlen. Как бы вы получили этот номер, и если бы он у вас был, зачем вам звонить в str [n] len? Это не то, для чего strnlen вообще.
Джим Балтер
1
+1 Хотя этот ответ несовершенен, потому что он не эквивалентен коду OP - strncpy NUL-pad и не завершается NUL, тогда как strcpy NUL-завершается и не NUL-pad, но он решает проблему, в отличие от смешные, невежественные комментарии выше.
Джим Балтер
4

Ответ с упаковкой правильный. Но есть проблема, я думаю, не была упомянута, если (len> = 100)

Ну, если бы Len был 100, мы скопировали бы 100 элементов, и у нас не было бы трейлинга \ 0. Это явно означало бы, что любая другая функция, зависящая от правильной завершенной строки, будет выходить далеко за пределы исходного массива.

ИМХО неразрешимая строка из C является ИМХО. Тебе лучше иметь некоторые ограничения до звонка, но даже это не поможет. Проверка границ отсутствует, поэтому переполнение буфера всегда возможно и, к сожалению, произойдет ....

Фридрих
источник
Строка проблематичной является разрешимой: просто использовать соответствующие функции. И. е. не strncpy() и друзья, а функции выделения памяти, как strdup()и друзья. Они соответствуют стандарту POSIX-2008, поэтому они достаточно портативны, хотя и недоступны в некоторых проприетарных системах.
cmaster - восстановить монику
«любая другая функция, зависящая от правильной завершенной строки» - bufferявляется локальной для этой функции и не используется в другом месте. В реальной программе нам нужно было бы изучить, как она используется ... иногда неправильное NUL-завершение является правильным (первоначальное использование strncpy состояло в создании 14-байтовых записей каталога UNIX - дополненных NUL и не заканчивающихся NUL). «Строка, проблематичная из C, ИМХО неразрешима» - в то время как C - это отвратительный язык, который превзошел намного лучшую технологию, безопасный код может быть написан на нем, если используется достаточно дисциплины.
Джим Балтер
Ваше наблюдение кажется мне ошибочным. if (len >= 100)является условием, когда проверка заканчивается неудачей , а не когда она проходит, что означает, что не существует случая, когда копируется ровно 100 байтов без терминатора NUL, поскольку эта длина включена в условие сбоя.
Патрик Робертс
@ cmaster. В этом случае вы не правы. Это не решаемо, потому что всегда можно написать за пределами границ. Да, это неопознанное поведение, но нет способа полностью его предотвратить.
Фридрих
@ Джим Балтер. Не важно. Я потенциально могу записать за пределы этого локального буфера, и поэтому всегда будет возможно повредить некоторые другие структуры данных.
Фридрих
3

Помимо проблем безопасности, связанных с вызовом strlenболее одного раза, обычно не следует использовать строковые методы для строк, длина которых точно известна [для большинства строковых функций существует только очень узкий случай, когда их следует использовать - для строк, для которых максимум длина может быть гарантирована, но точная длина не известна]. Как только длина входной строки известна, а длина выходного буфера известна, следует выяснить, насколько большой регион должен быть скопирован, а затем использовать его memcpy()для фактического выполнения рассматриваемого копирования. Хотя возможно, что это strcpyможет быть лучше, чем memcpy()при копировании строки длиной всего 1-3 байта, на многих платформах memcpy(), вероятно, будет более чем в два раза быстрее при работе с большими строками.

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

char *strdupe(char const *src)
{
  size_t len = strlen(src);
  char *dest = malloc(len+1);
  // Calculation can't wrap if string is in valid-size memory block
  if (!dest) return (OUT_OF_MEMORY(),(char*)0); 
  // OUT_OF_MEMORY is expected to halt; the return guards if it doesn't
  memcpy(dest, src, len);      
  dest[len]=0;
  return dest;
}

Обратите внимание, что последний оператор, как правило, может быть опущен, если memcpy обработал len+1байты, но если другой поток должен был изменить исходную строку, результатом может быть строка назначения, не оканчивающаяся NUL.

Supercat
источник
3
Можете ли вы объяснить проблемы безопасности, связанные с вызовом strlenболее одного раза ?
Богдан Александру
1
@BogdanAlexandru: После того, как кто-то вызвал strlenи предпринял какое-то действие, основанное на возвращенном значении (которое, по-видимому, было причиной его вызова в первую очередь), то повторный вызов any (1) всегда даст тот же ответ, что и первый, в этом случае это просто потраченная впустую работа, или (2) может иногда (потому что что-то другое - возможно, другой поток - изменило строку в это время) дать другой ответ, и в этом случае код, который делает некоторые вещи с длиной (например, выделение буфера) может принимать размер, отличный от кода, который делает другие вещи (копирование в буфер).
суперкат