From fb44a233a5be72d8d1cfd02e300db7de2b4bf428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eliseo=20Marti=CC=81nez?= Date: Fri, 20 Feb 2015 16:32:58 +0100 Subject: [PATCH] coverity/13777: String not null terminated: RI. Problem : String not null terminated @ 1543. Diagnostic : Real issue. Rationale : We are reading a struct block0, which contains some string fields, from a file, without checking for string fields to be correctly terminated. That could cause a buffer overrun if file has somehow been garbled. Resolution : Add string fields check for nul termination. Mark issue as intentional (there seems to be no way of teaching coverity about read_eintr being ok that way). Helped-by: oni-link --- src/nvim/fileio.c | 4 ++-- src/nvim/memline.c | 12 ++++++++++++ src/nvim/vim.h | 7 ++----- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/nvim/fileio.c b/src/nvim/fileio.c index 9d4c990f3a..799a6a2a50 100644 --- a/src/nvim/fileio.c +++ b/src/nvim/fileio.c @@ -7416,7 +7416,7 @@ long read_eintr(int fd, void *buf, size_t bufsize) long ret; for (;; ) { - ret = vim_read(fd, buf, bufsize); + ret = read(fd, buf, bufsize); if (ret >= 0 || errno != EINTR) break; } @@ -7435,7 +7435,7 @@ long write_eintr(int fd, void *buf, size_t bufsize) /* Repeat the write() so long it didn't fail, other than being interrupted * by a signal. */ while (ret < (long)bufsize) { - wlen = vim_write(fd, (char *)buf + ret, bufsize - ret); + wlen = write(fd, (char *)buf + ret, bufsize - ret); if (wlen < 0) { if (errno != EINTR) break; diff --git a/src/nvim/memline.c b/src/nvim/memline.c index 5a577c6378..d6d7d3db1a 100644 --- a/src/nvim/memline.c +++ b/src/nvim/memline.c @@ -54,6 +54,7 @@ #include "nvim/cursor.h" #include "nvim/eval.h" #include "nvim/fileio.h" +#include "nvim/func_attr.h" #include "nvim/main.h" #include "nvim/mark.h" #include "nvim/mbyte.h" @@ -630,6 +631,15 @@ static int ml_check_b0_id(ZERO_BL *b0p) return OK; } +/// Return true if all strings in b0 are correct (nul-terminated). +static bool ml_check_b0_strings(ZERO_BL *b0p) FUNC_ATTR_NONNULL_ALL +{ + return (memchr(b0p->b0_version, NUL, 10) + && memchr(b0p->b0_uname, NUL, B0_UNAME_SIZE) + && memchr(b0p->b0_hname, NUL, B0_HNAME_SIZE) + && memchr(b0p->b0_fname, NUL, B0_FNAME_SIZE_CRYPT)); +} + /* * Update the timestamp or the B0_SAME_DIR flag of the .swp file. */ @@ -1522,6 +1532,8 @@ static time_t swapfile_info(char_u *fname) MSG_PUTS(_(" [from Vim version 3.0]")); } else if (ml_check_b0_id(&b0) == FAIL) { MSG_PUTS(_(" [does not look like a Vim swap file]")); + } else if (!ml_check_b0_strings(&b0)) { + MSG_PUTS(_(" [garbled strings (not nul terminated)]")); } else { MSG_PUTS(_(" file name: ")); if (b0.b0_fname[0] == NUL) diff --git a/src/nvim/vim.h b/src/nvim/vim.h index 29d61dcbde..410c2602c8 100644 --- a/src/nvim/vim.h +++ b/src/nvim/vim.h @@ -322,13 +322,10 @@ enum { (size_t)(n)) #ifndef EINTR -# define read_eintr(fd, buf, count) vim_read((fd), (buf), (count)) -# define write_eintr(fd, buf, count) vim_write((fd), (buf), (count)) +# define read_eintr(fd, buf, count) read((fd), (buf), (count)) +# define write_eintr(fd, buf, count) write((fd), (buf), (count)) #endif -# define vim_read(fd, buf, count) read((fd), (char *)(buf), (size_t) (count)) -# define vim_write(fd, buf, count) write((fd), (char *)(buf), (size_t) (count)) - /* * Enums need a typecast to be used as array index (for Ultrix). */