¿Por qué este código es vulnerable a los ataques de desbordamiento de búfer?


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

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

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

Este código es vulnerable a un ataque de desbordamiento de búfer, y estoy tratando de averiguar por qué. Estoy pensando que tiene que ver con len ser declarado un short en lugar de un int, pero no estoy realmente seguro.

Alguna idea?

Author: Noam M, 2015-04-28

5 answers

En la mayoría de los compiladores el valor máximo de un unsigned short es 65535.

Cualquier valor por encima de eso se envuelve alrededor, por lo que 65536 se convierte en 0, y 65600 se convierte en 65.

Esto significa que las cadenas largas de la longitud correcta (por ejemplo, 65600) pasarán la comprobación y desbordarán el búfer.


Use size_t para almacenar el resultado de strlen(), no unsigned short, y compare len con una expresión que codifica directamente el tamaño de buffer. Así, por ejemplo:

char buffer[100];
size_t len = strlen(str);
if (len >= sizeof(buffer) / sizeof(buffer[0]))  return -1;
memcpy(buffer, str, len + 1);
 192
Author: orlp,
Warning: date(): Invalid date.timezone value 'Europe/Kyiv', we selected the timezone 'UTC' for now. in /var/www/agent_stack/data/www/ajaxhispano.com/template/agent.layouts/content.php on line 61
2015-04-30 17:35:55

El problema está aquí:

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

Si la cadena es mayor que la longitud del búfer de destino, strncpy todavía copiarla. Está basando el número de caracteres de la cadena como el número a copiar en lugar del tamaño del búfer. La forma correcta de hacer esto es la siguiente:

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

Lo que esto hace es limitar la cantidad de datos copiados al tamaño real del búfer menos uno para el carácter de terminación nulo. Luego establecemos el último byte en el búfer al null carácter como salvaguardia adicional. La razón de esto es porque strncpy copiará hasta n bytes, incluyendo el null de terminación, si strlen(str)

Espero que esto ayude.

EDITAR: Tras un examen adicional y la entrada de otros, una posible codificación para la función sigue:

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);
  }

Como ya sabemos la longitud de la cadena, podemos usar memcpy para copie la cadena de la ubicación a la que hace referencia str en el búfer. Tenga en cuenta que según la página de manual de strlen(3) (en un sistema FreeBSD 9.3), se indica lo siguiente:

 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.

Que interpreto como que la longitud de la cadena no incluye el null. Es por eso que copio len + 1 bytes para incluir el null, y la prueba comprueba para asegurarse de que la longitud

EDITAR: Resulta que el tamaño de algo comienza con 1 mientras que el acceso comienza con 0, por lo que el -2 anterior era incorrecto porque devolvería un error para cualquier cosa > 98 bytes pero debería ser > 99 bytes.

EDITAR: Aunque la respuesta sobre un corto sin signo es generalmente correcta ya que la longitud máxima que se puede representar es de 65.535 caracteres, realmente no importa porque si la cadena es más larga que eso, el valor se ajustará. Es como tomando 75,231 (que es 0x000125DF) y enmascarando los 16 bits superiores que le dan 9695 (0x000025DF). El único problema que veo con esto es que los primeros 100 caracteres pasan de 65,535 ya que la comprobación de longitud permitirá la copia, pero solo copiará hasta los primeros 100 caracteres de la cadena en todos los casos y terminará null la cadena. Así que incluso con el problema envolvente, el búfer todavía no se desbordará.

Esto puede o no plantear en sí mismo un riesgo de seguridad dependiendo de la contenido de la cadena y para qué la está usando. Si es solo texto recto que es legible por humanos, entonces generalmente no hay problema. Sólo tienes una cadena truncada. Sin embargo, si es algo así como una URL o incluso una secuencia de comandos SQL, podría tener un problema.

 27
Author: Daniel Rudy,
Warning: date(): Invalid date.timezone value 'Europe/Kyiv', we selected the timezone 'UTC' for now. in /var/www/agent_stack/data/www/ajaxhispano.com/template/agent.layouts/content.php on line 61
2015-05-21 08:41:44

Aunque esté usando strncpy, la longitud del corte sigue dependiendo del puntero de cadena pasado. No tiene idea de cuánto tiempo es esa cadena (la ubicación del terminador nulo en relación con el puntero, es decir). Así que llamar a strlen solo te abre a la vulnerabilidad. Si desea estar más seguro, utilice strnlen(str, 100).

El código completo corregido sería:

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;
}
 11
Author: Patrick Roberts,
Warning: date(): Invalid date.timezone value 'Europe/Kyiv', we selected the timezone 'UTC' for now. in /var/www/agent_stack/data/www/ajaxhispano.com/template/agent.layouts/content.php on line 61
2015-04-28 04:59:28

La respuesta con el envoltorio es correcta. Pero hay un problema que creo que no se mencionó if (len >= 100)

Bueno, si Len fuera 100 copiaríamos 100 elementos y no tendríamos final \0. Eso claramente significaría que cualquier otra función dependiendo de la cadena terminada adecuada caminaría mucho más allá de la matriz original.

La cadena problemática de C es insoluble en mi humilde opinión. Siempre será mejor que tengas algunos límites antes de la llamada, pero incluso eso no ayudará. No hay límites de verificación y así los desbordamientos de búfer siempre pueden y desafortunadamente sucederán....

 4
Author: Friedrich,
Warning: date(): Invalid date.timezone value 'Europe/Kyiv', we selected the timezone 'UTC' for now. in /var/www/agent_stack/data/www/ajaxhispano.com/template/agent.layouts/content.php on line 61
2015-04-28 05:30:28

Más allá de los problemas de seguridad que implica llamar a strlen más de una vez, generalmente no se deben usar métodos de cadena en cadenas cuya longitud se conoce con precisión [para la mayoría de las funciones de cadena, solo hay un caso muy estrecho donde se deben usar on en cadenas para las que se puede garantizar una longitud máxima, pero no se conoce la longitud precisa]. Una vez que se conoce la longitud de la cadena de entrada y se conoce la longitud del búfer de salida, uno debe averiguar qué tan grande debe copiarse una región y luego use memcpy() para realizar realmente la copia en cuestión. Aunque es posible que strcpy pueda superar a memcpy() al copiar una cadena de solo 1-3 bytes o algo así, en muchas plataformas memcpy() es probable que sea más del doble de rápido cuando se trata de cadenas más grandes.

Aunque hay algunas situaciones en las que la seguridad vendría a costa del rendimiento, esta es una situación en la que el enfoque seguro es también el más rápido. En algunos casos, puede ser razonable escribir código que no es seguro contra entradas de comportamiento extraño, si el código que suministra las entradas puede garantizar que se comportarán bien, y si protegerse contra entradas de mal comportamiento impediría el rendimiento. Garantizar que las longitudes de cadena solo se comprueben una vez mejora tanto el rendimiento como la seguridad de, aunque se puede hacer una cosa adicional para ayudar a proteger la seguridad incluso cuando se rastrea la longitud de la cadena manualmente: para cada cadena que se espera que tenga un null final, escriba el null final explícitamente en lugar de que esperar que la cadena de origen lo tenga. Por lo tanto, si uno estuviera escribiendo un strdup equivalente:

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;
}

Tenga en cuenta que la última instrucción generalmente se podría omitir si el memcpy había procesado len+1 bytes, pero si otro subproceso fuera modificar la cadena de origen, el resultado podría ser una cadena de destino no terminada en NUL.

 3
Author: supercat,
Warning: date(): Invalid date.timezone value 'Europe/Kyiv', we selected the timezone 'UTC' for now. in /var/www/agent_stack/data/www/ajaxhispano.com/template/agent.layouts/content.php on line 61
2015-04-29 15:36:02