From ca7a5eda221d53aa571836a6186ed117c804d702 Mon Sep 17 00:00:00 2001 From: Elie De Brauwer Date: Fri, 1 Mar 2013 19:37:39 +0100 Subject: integck.c: Fix buffer overflow in save_file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the problem above I've spend several hours waiting for the issue to appear, only to had the 'luck' that it was found in a file whose name was 256 bytes in length, resulting in the write to fail. Closer examination showed that the buffer to store the path was 256 bytes in length, but this buffer also includes /tmp and the read/write suffix and should be able to contain a filename which is up to 255 bytes (NAME_MAX in linux/limits.h) in size which is a bad fit. So that array is modified to FILENAME_MAX (stdio_lim.h) and some checking is added to truncate the filename should it cause an overflow. The following log shows the first patch in action (see the correct seed), and shows why this third patch is needed: integck:     File Data: integck:         Offset: 0  Size: 1  Seed: 5008310  R.Off: 0 integck:     1 writes integck:     ============================================ integck:     Write Info: integck:         Offset: 0  Size: 1  Seed: 5008310  R.Off: 0 integck:         Offset: 0  Size: 1  Seed: 8246352  R.Off: 0 integck:         Offset: 0  Size: 1  Seed: 5078796  R.Off: 0 integck:         Offset: 0  Size: 1  Seed: 2267087  R.Off: 0 integck:         Offset: 0  Size: 1  Seed: 3602680  R.Off: 0 integck:     5 writes or truncations integck:     ============================================ integck: Saving /tmp/yqcnfygfitaatyeyvffrguegcdttamcnyhowhgieljfuxfipiljsjcbluaeaghwyinkggommsbwnmvekihgnwgiibccpbwfrpxuxwkmnyghnutrudienngxwgorudbskedaaekiuiyqksfazrwzfwbfhzjjqoiulebtlpbfiuffmsnguqkjzqjqizimsmhbqqagaebjdhqwmzdxghiavtcxubegawlgtvstuqurkurpnrckjfkgostdtpg.integ.sav.readn integck: error!: condition 'w_fd != -1' failed in save_file() at integck.c:1445 integck: error 36 (File name too long) Signed-off-by: Elie De Brauwer Signed-off-by: Artem Bityutskiy --- tests/fs-tests/integrity/integck.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/fs-tests/integrity/integck.c b/tests/fs-tests/integrity/integck.c index 5ea3642..ee37a0d 100644 --- a/tests/fs-tests/integrity/integck.c +++ b/tests/fs-tests/integrity/integck.c @@ -32,11 +32,11 @@ #include #include #include +#include #include #include #include #include -#include #define PROGRAM_VERSION "1.1" #define PROGRAM_NAME "integck" @@ -1433,12 +1433,17 @@ static void save_file(int fd, struct file_info *file) int w_fd; struct write_info *w; char buf[IO_BUFFER_SIZE]; - char name[256]; + char name[FILENAME_MAX]; + const char * read_suffix = ".integ.sav.read"; + const char * write_suffix = ".integ.sav.written"; + size_t fname_len = strlen(get_file_name(file)); /* Open file to save contents to */ strcpy(name, "/tmp/"); - strcat(name, get_file_name(file)); - strcat(name, ".integ.sav.read"); + if (fname_len + strlen(read_suffix) > fsinfo.max_name_len) + fname_len = fsinfo.max_name_len - strlen(read_suffix); + strncat(name, get_file_name(file), fname_len); + strcat(name, read_suffix); normsg("Saving %sn", name); w_fd = open(name, O_CREAT | O_WRONLY, 0777); CHECK(w_fd != -1); @@ -1457,8 +1462,10 @@ static void save_file(int fd, struct file_info *file) /* Open file to save contents to */ strcpy(name, "/tmp/"); - strcat(name, get_file_name(file)); - strcat(name, ".integ.sav.written"); + if (fname_len + strlen(write_suffix) > fsinfo.max_name_len) + fname_len = fsinfo.max_name_len - strlen(write_suffix); + strncat(name, get_file_name(file), fname_len); + strcat(name, write_suffix); normsg("Saving %s", name); w_fd = open(name, O_CREAT | O_WRONLY, 0777); CHECK(w_fd != -1); -- cgit v1.2.3