Как этот фрагмент C может быть написан более безопасно?

Question

У меня есть следующий фрагмент кода на C, и я должен определить ошибку и предложить более безопасный способ ее написания:

char somestring[] = "Send money!\n";
char *copy;

copy = (char *) malloc(strlen(somestring));

strcpy(copy, somestring);
printf(copy);

Итак, ошибка в том, что strlen игнорирует завершающий '\0' строки и, следовательно, ему не будет выделено достаточно памяти для копии, но я не уверен, к чему они стремятся написать более безопасно?

Я мог бы просто использовать malloc(strlen(somestring)+1)) Я предполагаю, но я думаю, что должен быть лучший способ, чем это?

---

РЕДАКТИРОВАТЬ: ОК, я принял ответ, я подозреваю, что от нас не ожидают решения strdup, поскольку оно не является частью ANSI C. Это кажется довольно субъективным вопросом, поэтому Я не уверен, что то, что я принял, на самом деле лучшее. В любом случае, спасибо за все ответы.

Answer 1

Я не могу комментировать ответы выше, но в дополнение к проверке код возврата и использование strncpy, вы никогда не должны делать:

printf(string)

Но используйте:

printf("%s", string);

ref: http://en.wikipedia.org/wiki/Format_string_attack

Answer 2

char somestring[] = "Send money!\n";
char *copy = strdup(something);

if (copy == NULL) {
    // error
}

или просто поместите эту логику в отдельную функцию xstrdup :

char * xstrdup(const char *src) 
{
    char *copy = strdup(src);

    if (copy == NULL) {
       abort();
    }

    return copy;
}

Answer 3

char   somestring[] = "Send money!\n";
char   *copy;
size_t copysize;

copysize = strlen(somestring)+1;
copy = (char *) malloc(copysize);
if (copy == NULL)
    bail("Oh noes!\n");

strncpy(copy, somestring, copysize);
printf("%s", copy);

Отмеченные различия выше:

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

EDIT:

Для тех, кто думает, что я должен использовать strdup() ... это работает, только если вы примете очень узкое представление вопроса Это не только глупо, но и дает еще лучший ответ:

char somestring[] = "Send money!\n";
char *copy = somestring;
printf(copy);

Если вы собираетесь быть тупым, по крайней мере, хорошо это делать.

Answer 4

Ick ... используйте strdup(), как все говорили, и пишите сами, если нужно. Поскольку у вас есть время подумать об этом сейчас ... посмотрите 25 самых опасных ошибок программирования на митре , затем подумайте, почему фраза printf(copy) не должна никогда появляться в коде. Это прямо там с malloc(strlen(str)) с точки зрения абсолютной злости, не говоря уже о головной боли, связанной с поиском причин, почему это вызывает много горя, когда копирование что-то вроде "%s%n" ...

Answer 5

1. strlen + 1, для терминатора \ 0
2. malloc может завершиться ошибкой; всегда проверяйте возвращаемое значение malloc

Answer 6

, чтобы добавить больше способов Адриана МакКарти сделать более безопасный код,

Используйте статический анализатор кода, они очень хорошо умеют находить ошибки такого типа

Answer 7

Способы сделать код более безопасным (и более правильным).

1. Не делайте ненужную копию. Из примера нет очевидного требования, что вам действительно нужно скопировать somestring. Вы можете вывести его напрямую.
2. Если вам нужно сделать копию строки, напишите функцию для этого (или используйте strdup, если он у вас есть). Тогда вам нужно только сделать это правильно в одном месте.
3. По возможности инициализируйте указатель на копию сразу же после ее объявления.
4. Не забудьте выделить место для нулевого терминатора.
5. Не забудьте проверить возвращаемое значение из malloc.
6. Не забудьте освободить malloc 'ed память.
7. Не вызывайте printf со строкой ненадежного формата. Используйте printf("%s", copy) или puts(copy).
8. Используйте объектно-ориентированный язык со строковым классом или любой язык со встроенной поддержкой строк, чтобы избежать большинства этих проблем.

Answer 8

Я бы прокомментировал предыдущие решения, но мне не хватает представителя. Использование strncpy здесь также неправильно, как использование strcpy (так как нет абсолютно никакого риска переполнения) В есть функция с именем memcpy , и она предназначена именно для этого. Это не только значительно быстрее, но и правильная функция, используемая для копирования строк известной длины в стандарте C.

Из принятого ответа:

char   somestring[] = "Send money!\n";
char   *copy;
size_t copysize;

copysize = strlen(somestring)+1;
copy = (char *) malloc(copysize);
if (copy == NULL)
    bail("Oh noes!\n");

memcpy(copy, somestring, copysize); /* You don't use str* functions for this! */
printf("%s", copy);

Answer 9

Лучший способ написать это более безопасно, если бы кто-то действительно интересовался такими вещами, это написать это на Аде.

somestring : constant string := "Send money!";
declare
   copy : constant string := somestring;
begin
   put_line (somestring);
end;

Тот же результат, так в чем же различия?

• Все это делается в стеке (без указателей). Распределение автоматический и безопасный.
• Все автоматически проверяется по дальности, поэтому нет шансов переполнения буфера подвиги
• Обе строки являются константами, так что нет шансов испортить и модифицируя их.
• Вероятно, это будет на быстрее , чем C, не только из-за отсутствия динамического выделения, но и потому, что нет дополнительного сканирования строки, требуемой strlen ().

Обратите внимание, что в Аде "строка" не является какой-то специальной динамической конструкцией. Это встроенный массив символов. Тем не менее, при объявлении массива Ada можно определить по массиву, который вы им присваиваете.

Answer 10

Более безопасный способ - использовать strncpy вместо strcpy. Эта функция принимает третий аргумент: длину копируемой строки. Это решение не выходит за рамки ANSI C, поэтому оно будет работать во всех средах (тогда как другие методы могут работать только в POSIX-совместимых системах).

char somestring[] = "Send money!\n";
char *copy;

copy = (char *) malloc(strlen(somestring));

strncpy(copy, somestring, strlen(somestring));
printf(copy);