From 392702ee2c88d7d8aaff25f7a84acb73606f9094 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 9 Feb 2015 23:41:13 -0500 Subject: allocations: test for overflow of requested size Introduce some helper macros to test integer overflow from arithmetic and set error message appropriately. --- src/fileops.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src/fileops.c') diff --git a/src/fileops.c b/src/fileops.c index eddd5a804..eb24013e8 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -127,6 +127,7 @@ int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len) git_buf_clear(buf); + GITERR_CHECK_ALLOC_ADD(len, 1); if (git_buf_grow(buf, len + 1) < 0) return -1; @@ -708,7 +709,10 @@ static int cp_link(const char *from, const char *to, size_t link_size) { int error = 0; ssize_t read_len; - char *link_data = git__malloc(link_size + 1); + char *link_data; + + GITERR_CHECK_ALLOC_ADD(link_size, 1); + link_data = git__malloc(link_size + 1); GITERR_CHECK_ALLOC(link_data); read_len = p_readlink(from, link_data, link_size); -- cgit v1.2.1 From 8d534b475829edf87c8126fc4ae30f593172f317 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 11 Feb 2015 13:01:00 -0500 Subject: p_read: ensure requested len is ssize_t Ensure that the given length to `p_read` is of ssize_t and ensure that callers test the return as if it were an `ssize_t`. --- src/fileops.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/fileops.c') diff --git a/src/fileops.c b/src/fileops.c index eb24013e8..420ed70a2 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -127,6 +127,11 @@ int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len) git_buf_clear(buf); + if (!git__is_ssizet(len)) { + giterr_set(GITERR_INVALID, "Read too large."); + return -1; + } + GITERR_CHECK_ALLOC_ADD(len, 1); if (git_buf_grow(buf, len + 1) < 0) return -1; -- cgit v1.2.1 From f1453c59b2afb9dab43281bfe9f1ba34cf6e0d02 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 12 Feb 2015 12:19:37 -0500 Subject: Make our overflow check look more like gcc/clang's Make our overflow checking look more like gcc and clang's, so that we can substitute it out with the compiler instrinsics on platforms that support it. This means dropping the ability to pass `NULL` as an out parameter. As a result, the macros also get updated to reflect this as well. --- src/fileops.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'src/fileops.c') diff --git a/src/fileops.c b/src/fileops.c index 420ed70a2..09a8f5d4a 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -124,6 +124,7 @@ mode_t git_futils_canonical_mode(mode_t raw_mode) int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len) { ssize_t read_size = 0; + size_t alloc_len; git_buf_clear(buf); @@ -132,8 +133,8 @@ int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len) return -1; } - GITERR_CHECK_ALLOC_ADD(len, 1); - if (git_buf_grow(buf, len + 1) < 0) + GITERR_CHECK_ALLOC_ADD(&alloc_len, len, 1); + if (git_buf_grow(buf, alloc_len) < 0) return -1; /* p_read loops internally to read len bytes */ @@ -455,7 +456,13 @@ int git_futils_mkdir_ext( } if (opts->dir_map && opts->pool) { - char *cache_path = git_pool_malloc(opts->pool, make_path.size + 1); + char *cache_path; + size_t alloc_size; + + GITERR_CHECK_ALLOC_ADD(&alloc_size, make_path.size, 1); + if (!git__is_uint32(alloc_size)) + return -1; + cache_path = git_pool_malloc(opts->pool, (uint32_t)alloc_size); GITERR_CHECK_ALLOC(cache_path); memcpy(cache_path, make_path.ptr, make_path.size + 1); @@ -715,9 +722,10 @@ static int cp_link(const char *from, const char *to, size_t link_size) int error = 0; ssize_t read_len; char *link_data; + size_t alloc_size; - GITERR_CHECK_ALLOC_ADD(link_size, 1); - link_data = git__malloc(link_size + 1); + GITERR_CHECK_ALLOC_ADD(&alloc_size, link_size, 1); + link_data = git__malloc(alloc_size); GITERR_CHECK_ALLOC(link_data); read_len = p_readlink(from, link_data, link_size); -- cgit v1.2.1