Каковы ошибки в этой функции из вопроса об интервью Microsoft?

Question

Мне задали этот вопрос в письменном интервью для MS:

Найдите ошибки в программе ниже, которая должна вернуть новую строку с добавленным \n.

char* AddnewlinetoString(char *s)
{
  char buffer[1024];
  strcpy(buffer,s);
  buffer[strlen(s)-1] = '\n';
  return buffer;
}

Я попытался написать код для себя и смог заставить его работать, сделав буферную переменную глобальной и получив buffer[strlen(s)] = '\n'.Но не знал, что в нем было много других ошибок.

Answer 1

Я вижу несколько:

Длина входной строки не проверена.

Что если strlen(s) > 1023? Вы можете поместить строку длиной не более 1023 в буфер.

Перезаписать последний символ с помощью \n

Вы перезаписываете последний символ новой строкой. Ваш \n должен идти туда, где раньше был \0, и вам нужно добавить новый \0 после \n

Буфер переменных является локальным для функции, и вы возвращаете его адрес.

Память для буфера выделяется в стеке, и как только функция возвращается, эта память освобождается.

Я бы сделал:

char* AddnewlinetoString(char *s) {

  size_t buffLen = strlen(s) + 2; // +1 for '\n' +1 for '\0'
  char *buffer = malloc(buffLen); 
  if(!buffer) {
   fprintf(stderr,"Error allocting\n");
   exit(1);
  }
  strcpy(buffer,s);
  buffer[buffLen-2] = '\n';
  buffer[buffLen-1] = 0;
  return buffer;
}

Answer 2

Вот версия C ++ без ошибок:

std::string AddnewlinetoString(std::string const& s)
{
    return s + "\n";
}

А вот как я мог бы написать это в C ++ 0x:

std::string AddnewlinetoString(std::string s)
{
    return std::move(s += "\n");
}

Answer 3

1. нет ограничений в strcpy, лучше использовать strncpy.
2. вы копируете в статический буфер и возвращаете указатель.

Answer 4

Я бы также добавил, что название метода должно соответствовать шаблону, и каждое слово должно начинаться с заглавной буквы:

char* AddNewlineToString(char *s)
{
}

пс.Спасибо, Конрад, я изменил название метода, как вы предложили

Answer 5

3 вещи

   int len = strlen(s);
   char* buffer = (char*) malloc (len + 2);   // 1
   strcpy(buffer,s);
   buffer[len] = '\n';           // 2 
   buffer[len+1] = '\0';         // 3
   return buffer;

Редактировать: На основе комментариев

Answer 6

Основная проблема с этим кодом заключается в том, что он уязвим для эксплойта переполнения буфера в стеке . Это классический пример.

Как правило, входной символ * может быть длиннее 1024 байта; затем эти дополнительные байты перезапишут стек, позволяя злоумышленнику изменить указатель возврата функции, чтобы он указывал на свой вредоносный код. Ваша программа невольно выполнит вредоносный код.

Можно ожидать, что Microsoft позаботится об этих видах эксплойтов, поскольку Red Worm Code классно использовал переполнение стекового буфера для атаки на сотни тысяч компьютеров, работающих под управлением программного обеспечения веб-сервера IIS в 2001 году.

Answer 7

Вот исправленная версия (в вики-сообществе я ничего не пропустил)

// caller must free() returned buffer string!
char* AddnewlinetoString(char *s)
{
  size_t len;
  char * buffer;

  if (s == NULL)
    s = "";

  len = strlen(s);
  buffer = malloc(len+2);
  if (buffer == NULL)
    abort();
  strcpy(buffer,s);
  buffer[len] = '\n';
  buffer[len+1] = 0;
  return buffer;
}

Как упоминает Тони, s может быть допустимым адресом, но все же быть искаженной c-строкой без нулевых байтов. Функция может закончить чтение до тех пор, пока не вызовет ошибку или другую ужасную вещь. Хотя это все еще идиоматический C, большинство людей предпочитают подсчитанные строки (а не завершенные нулем.)

// caller must free() returned buffer string!
char* AddnewlinetoStringN(char *s, size_t len)
{
  char * buffer;

  if (s == NULL)
    s = "";

  buffer = malloc(len+1); // only add 1 byte, since there's no need for the nul
  if (buffer == NULL)
    abort();
  strncpy(buffer,s,len);
  buffer[len] = '\n';
  return buffer;
}

Answer 8

Нет необходимости в указателе возврата.Изменить входящий указатель.

int len = strlen(s); s[len] = '\n'; s[len + 1] = '\0';

Answer 9

Здесь идет более простой путь:

char* AddnewlinetoString(char *s)
{
  char buffer[strlen(s)+1];
  snprintf(buffer,strlen(s),"%s\n",s);
  return buffer;
}

Answer 10

Может быть сделано очень просто, используя strdup:

char* AddnewlinetoString(char *s) {<br> char *buffer = strdup(s);<br> buffer[strlen(s)-1] = '\n';<br> return buffer;<br> }