Problema con strncpy

[vescapam]

Estoy analizado este programa en C que me da un overflow al introducir como argumento un archivo con más de 32 caracteres. No se si es porque utilizo mal la función strncopy. He marcado en negrita lás líneas que creo que son las que tengo mal. ¿Alguna idea? Muchas gracias.


#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* Returns whether a given file is a virus or not. Said file must be honest
about its intentions with regard the computer. */
char isAVirus(const char *path) {
FILE * f;
char signature[] = "I'm a virus";
char b[sizeof(signature)];

if ((f = fopen(path, "rb")) == NULL) {
perror(__FILE__);
exit(EXIT_FAILURE);
}

if (fread(b, 1, sizeof(signature), f) < sizeof(signature) && ! feof(f)) {
perror(__FILE__);
exit(EXIT_FAILURE);
}

fclose(f);

return memcmp(b, signature, strlen(signature)) == 0;
}

#define MAX_REPORT_FILENAME 32

/* Report item structure */
struct reportItem {
char filename[MAX_REPORT_FILENAME];
char isAVirus;
struct reportItem * next;
};

/* Adds one item to a report */
void appendReportItem(struct reportItem ** report, struct reportItem * item) {
/* Duplicate current item */
struct reportItem * itemCopy =
(struct reportItem *) malloc(sizeof(struct reportItem));

memcpy(itemCopy, item, sizeof(struct reportItem));

/* Add into the linked list */
itemCopy->next = *report;
*report = itemCopy;
}

/* Frees the memory allocated in a report */
void freeFullReport(struct reportItem ** report) {
struct reportItem * next = *report;

while(next != NULL) {
struct reportItem * current = next;
next = current->next;
free(current);
}

*report = NULL;
}

/* Prints a nice report of what has been found */
void printReport(struct reportItem * report) {
struct reportItem * next = report;
int counter = 0;
const char * dashes39 = "---------------------------------------";

/* Report header */
puts(" ___ _______ __ __ _______");
puts(" / | / _____/ / | / | / _____/");
puts(" / /| | / / / | / | / /____ ");
puts(" / /_| | / / / /| |/ /| | / _____/ ");
puts(" / ___ | / /____ / / |___/ | | / /____ ");
puts(" /_/ |_| /______/ /_/ |_| /______/ ");
puts(" [SCAN] ");

printf(" Report:\n");
printf(" %s%s\n", dashes39, dashes39);

/* Print entries */
while(next != NULL) {
printf(" %s [%s]\n", next->filename,
next->isAVirus ? "Virus Found!" : "OK");

next = next->next;
counter++;
}

/* Report footer */
printf(" %s%s\n", dashes39, dashes39);
printf(" %d elements scanned\n\n", counter);
}

int main(int argc, char **argv) {
int i;
struct reportItem * fullReport = NULL;

/* Scan each file of the command line and create a report item into the
full report */
for (i = 1; i < argc; i++) {
/* Create a report item */
struct reportItem currentItem;

/* Scan this file */
currentItem.isAVirus = isAVirus(argv[i]);

/* Copy the filename into the limited size field. If the filename is
larger than the allocated space, only show in the final report a part
of the filename. */
strncpy(currentItem.filename, argv[i], sizeof(currentItem.filename));

/* Since strncpy does not always copy the end '\0', if needed just
add one. */
if (strlen(argv[i]) > sizeof(currentItem.filename)) {
/* POSIBLE ERROR */
currentItem.filename[sizeof(currentItem.filename)] = '\0';
}

/* Append this report item to the full report */
appendReportItem(&fullReport, &currentItem);
}

/* Print the report */
printReport(fullReport);

/* Free memory */
freeFullReport(&fullReport);

/* We finished */
return 0;
}

[sam90]

Lo que pasa es que definiste una costante

#define MAX_REPORT_FILENAME 32

Que te limita el tamaño de los archivo.... aumenta ese numero y todo va mejorar.

[vescapam]

Muchas gracias Sam90, se lo de la constante, a lo que me refería es si cuando copias una cadena con strncopy es correcto añadir al final \0 o no. Creo que al hacerlo mal sea esta la razón del overflow.

Sabe alguien o tu mismo si esto está bien. Muchas gracias.


strncpy(currentItem.filename, argv[i], sizeof(currentItem.filename));

if (strlen(argv[i]) > sizeof(currentItem.filename)) {
currentItem.filename[sizeof(currentItem.filename)] = '\0';
}

[sam90]

Eso deberia ser asi:

currentItem.filename[sizeof(currentItem.filename) -1 ] = '\0';

Ya que los array van desde 0 hasta N-1

en tu caso de 0 a 31...

[vescapam]

Muchas gracias Sam90 eres un monstruo, efectivamente ese era el problema.