Desbordamiento de búfer debido a strlen, strcpy, strcat

1

Soy nuevo en la revisión de código seguro. Sé que strlen calculará la longitud hasta que encuentre un carácter nulo. Esto es parte de un código más grande.

char* executeMount(char* password, char* path, int unmountOrMount)
 {
 char* catString = new char[(strlen("echo|set /p password=") + strlen(password) + strlen(" | runas /user:administrator \"mountvol ") + strlen(path) + 1)];
 strcpy(catString, "echo|set /p password=");
 strcat(catString, password);
 strcat(catString, " | runas /user:administrator \"mountvol ");
 strcat(catString, path);
 //Equivalent command: system("echo|set /p password=" + adminPassword + " | runas /user:administrator mountvol" + path);
 return catString;
 }

¿Devolverá catString a un desbordamiento de búfer?

Strlen calculará todas las cadenas hasta que alcance un terminador nulo. Entonces catString es la longitud total de todas las cadenas + 1 (para el terminador nulo). strcpy copiará echo | set / p password="+ terminador nulo. El siguiente strcat eliminará el terminador nulo y concatenará la" contraseña "pero el strcat subsiguiente no eliminará el terminador nulo que conduce al desbordamiento del búfer. ¿Mi razonamiento es correcto? ¿Es este código? vulnerable a cualquier otra amenaza?

    
pregunta Rochelle 08.01.2018 - 12:55
fuente

2 respuestas

1
  

¿Devolverá catString a un desbordamiento del búfer debido a un strlen incorrecto?

Desde un vistazo rápido, la longitud de la cadena se ve bien. El único riesgo parece ser si los parámetros no se terminan en nulo, ya que esto provocaría que la siguiente contraseña / ruta se descargue en la línea de comandos.

  

¿Hay otras amenazas en este código?

Este código tiene muchos problemas, pero el desbordamiento del búfer debido a la longitud de asignación de catString no es uno de ellos. ¿Qué sucede cuando el usuario ingresa una ruta válida seguida de "; rmdir C: \ Windows / s "? O cualquier otro comando.

Como @Joe señala esto, probablemente debería usar std :: string en lugar de matrices de caracteres (incluida la devolución de una). Si va a utilizar matrices de caracteres, los punteros deben marcarse como const para forzar que no se modifiquen dentro de la función y permitir las optimizaciones del compilador. Contraseña & la ruta debe citarse y escaparse correctamente para detener la inyección de comandos (y permitir contraseñas con espacios y caracteres especiales). También creo que deberías usar permisos para permitir que la aplicación llame directamente a mountvol y no tener que pasarle una contraseña de cuenta de administrador.

    
respondido por el Hector 08.01.2018 - 13:47
fuente
1

Estás usando C ++ (porque no podrías usar new char si no lo estuvieras); Realmente deberías estar usando std::string aquí, incluido el valor de retorno. Luego, puede eliminar la referencia .c_str para obtener un puntero const char * si lo necesita en su código ascendente.

Sin embargo, no solo quieres devolver std::string.c_str desde tu función; El std::string quedará fuera del alcance y estará invocando un comportamiento indefinido.

Eso evita tener que lidiar con cualquiera de estas cosas de conteo de caracteres.

    
respondido por el Joe 08.01.2018 - 13:55
fuente

Lea otras preguntas en las etiquetas