Overflow del buffer a causa di strlen, strcpy, strcat

Question

Overflow del buffer a causa di strlen, strcpy, strcat

#1 da (1 voti)
#2 da (1 voti)

1

Sono nuovo per garantire la revisione del codice. So che strlen calcolerà la lunghezza finché non troverà un carattere nullo. Questa è una parte di un codice più 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;
 }

Restituirà lead di catString a un overflow del buffer?

Strlen calcolerà tutte le stringhe fino a raggiungere un terminatore nullo. Quindi catString è la lunghezza totale di tutte le stringhe + 1 (per il terminatore null). strcpy copierà echo | set / p password="+ terminatore null. Seguendo strcat rimuoverà il terminatore null e concatenare" password "ma lo strcat successivo non rimuoverà il terminatore null che porta al buffer overflow? Il mio pensiero è corretto? vulnerabile a qualsiasi altra minaccia?

c++ source-code buffer-overflow code-review

posta Rochelle 08.01.2018 - 12:55

fonte

2 risposte

Leggi altre domande sui tag c++ source-code buffer-overflow code-review

XMLHttpRequest CSRF non riesce con CORS consentito Come posso migliorare la sicurezza delle mie filiali protette in GitHub / GitLab?

score 1 · Answer 1

Will return catString lead to a buffer overflow due to the incorrect strlen?

Da una rapida panoramica la lunghezza della stringa sembra OK. L'unico rischio sembra essere se i parametri non sono nulli terminati - lì porterebbe a ciò che segue la password / il percorso che viene scaricato nella riga di comando.

Are there any other threats in this code?

Ci sono molti problemi con questo codice - ma l'overflow del buffer a causa della lunghezza dell'allocazione catString non è uno di questi. Cosa accade quando l'utente inserisce un percorso valido seguito da "; rmdir C: \ Windows / S"? O qualsiasi altro comando.

Come @Joe sottolinea che probabilmente dovresti usare std :: string piuttosto che array di caratteri (incluso restituirne uno). Se si utilizzano gli array di caratteri, i puntatori devono essere contrassegnati come const per imporre di non modificarli all'interno della funzione e consentire ottimizzazioni del compilatore. Password e amp; il percorso deve essere opportunamente quotato ed eseguito l'escape per interrompere l'iniezione del comando (e consentire password con spazi e caratteri speciali). Penso inoltre che dovresti usare le autorizzazioni per consentire all'applicazione di chiamare mountvol direttamente e non dover passare una password di account amministratore.

score 1 · Answer 2

Stai usando C ++ (perché non potresti usare new char se non lo fossi); dovresti davvero usare std::string qui, incluso come valore di ritorno. Puoi quindi dereferenziare .c_str per ottenere un puntatore const char * se è necessario nel codice upstream.

Tuttavia, non vuoi semplicemente restituire std::string.c_str dalla tua funzione; std::string uscirà dall'ambito e invocherai un comportamento indefinito.

Questo evita di dover gestire qualsiasi parte di questo conteggio di caratteri.