buffer: don't allow growing borrowed buffers

When we don't own a buffer (asize=0) we currently allow the usage of
grow to copy the memory into a buffer we do own. This muddles the
meaning of grow, and lets us be a bit cavalier with ownership semantics.

Don't allow this any more. Usage of grow should be restricted to buffers
which we know own their own memory. If unsure, we must not attempt to
modify it.

diff --git a/src/buffer.c b/src/buffer.c
index f633c5e02fccaa82daff51079bd8a756d7b022dd..c066d8e84b3d300f5d423a0234a8a1ca999a5c84 100644 (file)
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -33,7 +33,7 @@ void git_buf_init(git_buf *buf, size_t initial_size)
 }
 
 int git_buf_try_grow(
-       git_buf *buf, size_t target_size, bool mark_oom, bool preserve_external)
+       git_buf *buf, size_t target_size, bool mark_oom)
 {
        char *new_ptr;
        size_t new_size;
@@ -41,6 +41,9 @@ int git_buf_try_grow(
        if (buf->ptr == git_buf__oom)
                return -1;
 
+       if (buf->asize == 0 && buf->size != 0)
+               return GIT_EINVALIDSPEC;
+
        if (!target_size)
                target_size = buf->size;
 
@@ -82,9 +85,6 @@ int git_buf_try_grow(
                return -1;
        }
 
-       if (preserve_external && !buf->asize && buf->ptr != NULL && buf->size > 0)
-               memcpy(new_ptr, buf->ptr, min(buf->size, new_size));
-
        buf->asize = new_size;
        buf->ptr   = new_ptr;
 
@@ -98,7 +98,7 @@ int git_buf_try_grow(
 
 int git_buf_grow(git_buf *buffer, size_t target_size)
 {
-       return git_buf_try_grow(buffer, target_size, true, true);
+       return git_buf_try_grow(buffer, target_size, true);
 }
 
 int git_buf_grow_by(git_buf *buffer, size_t additional_size)
@@ -110,7 +110,7 @@ int git_buf_grow_by(git_buf *buffer, size_t additional_size)
                return -1;
        }
 
-       return git_buf_try_grow(buffer, newsize, true, true);   
+       return git_buf_try_grow(buffer, newsize, true);
 }
 
 void git_buf_free(git_buf *buf)

diff --git a/src/buffer.h b/src/buffer.h
index 093ed9b60585d5b99c2a242c726702669dc2d107..e46ee5dd748bb2c94b65d5171eaf5c555cd4be69 100644 (file)
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -59,7 +59,7 @@ extern int git_buf_grow_by(git_buf *buffer, size_t additional_size);
  * into the newly allocated buffer.
  */
 extern int git_buf_try_grow(
-       git_buf *buf, size_t target_size, bool mark_oom, bool preserve_external);
+       git_buf *buf, size_t target_size, bool mark_oom);
 
 /**
  * Sanitizes git_buf structures provided from user input.  Users of the

diff --git a/src/path.c b/src/path.c
index c2c90e48d78c49ad2b9d57f182a9445c94137a7a..2558058dd8bb58880556cbc5950beeb27c1211a2 100644 (file)
--- a/src/path.c
+++ b/src/path.c
@@ -640,7 +640,7 @@ static bool _check_dir_contents(
        /* leave base valid even if we could not make space for subdir */
        if (GIT_ADD_SIZET_OVERFLOW(&alloc_size, dir_size, sub_size) ||
                GIT_ADD_SIZET_OVERFLOW(&alloc_size, alloc_size, 2) ||
-               git_buf_try_grow(dir, alloc_size, false, false) < 0)
+               git_buf_try_grow(dir, alloc_size, false) < 0)
                return false;
 
        /* save excursion */
@@ -847,7 +847,7 @@ int git_path_make_relative(git_buf *path, const char *parent)
 
        /* save the offset as we might realllocate the pointer */
        offset = p - path->ptr;
-       if (git_buf_try_grow(path, alloclen, 1, 0) < 0)
+       if (git_buf_try_grow(path, alloclen, 1) < 0)
                return -1;
        p = path->ptr + offset;
 

diff --git a/tests/core/buffer.c b/tests/core/buffer.c
index fef37f8f78010a483e113c10ff68c4f91bc49747..cc2d7bbcccc9b8e4eee673a9e3d2cfe9644614d2 100644 (file)
--- a/tests/core/buffer.c
+++ b/tests/core/buffer.c
@@ -1153,3 +1153,16 @@ void test_core_buffer__lf_and_crlf_conversions(void)
        git_buf_free(&src);
        git_buf_free(&tgt);
 }
+
+void test_core_buffer__dont_grow_borrowed(void)
+{
+       const char *somestring = "blah blah";
+       git_buf buf = GIT_BUF_INIT;
+
+       git_buf_attach_notowned(&buf, somestring, strlen(somestring) + 1);
+       cl_assert_equal_p(somestring, buf.ptr);
+       cl_assert_equal_i(0, buf.asize);
+       cl_assert_equal_i(strlen(somestring) + 1, buf.size);
+
+       cl_git_fail_with(GIT_EINVALIDSPEC, git_buf_grow(&buf, 1024));
+}