Merge pull request #5232 from pks-t/pks/buffer-ensure-size-oom

buffer: fix writes into out-of-memory buffers

src/buffer.c

@@ -18,7 +18,8 @@ char git_buf__initbuf[1];
 char git_buf__oom[1];
 
 #define ENSURE_SIZE(b, d) \
-	if ((d) > (b)->asize && git_buf_grow((b), (d)) < 0)\
+	if ((b)->ptr == git_buf__oom || \
+	    ((d) > (b)->asize && git_buf_grow((b), (d)) < 0))\
 		return -1;
 
 
@@ -58,20 +59,26 @@ int git_buf_try_grow(
 		new_ptr = NULL;
 	} else {
 		new_size = buf->asize;
+		/*
+		 * Grow the allocated buffer by 1.5 to allow
+		 * re-use of memory holes resulting from the
+		 * realloc. If this is still too small, then just
+		 * use the target size.
+		 */
+		if ((new_size = (new_size << 1) - (new_size >> 1)) < target_size)
+			new_size = target_size;
 		new_ptr = buf->ptr;
 	}
 
-	/* grow the buffer size by 1.5, until it's big enough
-	 * to fit our target size */
-	while (new_size < target_size)
-		new_size = (new_size << 1) - (new_size >> 1);
-
 	/* round allocation up to multiple of 8 */
 	new_size = (new_size + 7) & ~7;
 
 	if (new_size < buf->size) {
-		if (mark_oom)
+		if (mark_oom) {
+			if (buf->ptr && buf->ptr != git_buf__initbuf)
+				git__free(buf->ptr);
 			buf->ptr = git_buf__oom;
+		}
 
 		git_error_set_oom();
 		return -1;

tests/core/buffer.c

@@ -231,9 +231,9 @@ check_buf_append(
 	git_buf_puts(&tgt, data_b);
 	cl_assert(git_buf_oom(&tgt) == 0);
 	cl_assert_equal_s(expected_data, git_buf_cstr(&tgt));
-	cl_assert(tgt.size == expected_size);
+	cl_assert_equal_i(tgt.size, expected_size);
 	if (expected_asize > 0)
-		cl_assert(tgt.asize == expected_asize);
+		cl_assert_equal_i(tgt.asize, expected_asize);
 
 	git_buf_dispose(&tgt);
 }
@@ -308,7 +308,7 @@ void test_core_buffer__5(void)
 					 REP16("x") REP16("o"), 32, 40);
 
 	check_buf_append(test_4096, "", test_4096, 4096, 4104);
-	check_buf_append(test_4096, test_4096, test_8192, 8192, 9240);
+	check_buf_append(test_4096, test_4096, test_8192, 8192, 8200);
 
 	/* check sequences of appends */
 	check_buf_append_abc("a", "b", "c",
@@ -1204,3 +1204,39 @@ void test_core_buffer__dont_grow_borrowed(void)
 
 	cl_git_fail_with(GIT_EINVALID, git_buf_grow(&buf, 1024));
 }
+
+void test_core_buffer__dont_hit_infinite_loop_when_resizing(void)
+{
+	git_buf buf = GIT_BUF_INIT;
+
+	cl_git_pass(git_buf_puts(&buf, "foobar"));
+	/*
+	 * We do not care whether this succeeds or fails, which
+	 * would depend on platform-specific allocation
+	 * semantics. We only want to know that the function
+	 * actually returns.
+	 */
+	(void)git_buf_try_grow(&buf, SIZE_MAX, true);
+
+	git_buf_dispose(&buf);
+}
+
+void test_core_buffer__avoid_printing_into_oom_buffer(void)
+{
+	git_buf buf = GIT_BUF_INIT;
+
+	/* Emulate OOM situation with a previous allocation */
+	buf.asize = 8;
+	buf.ptr = git_buf__oom;
+
+	/*
+	 * Print the same string again. As the buffer still has
+	 * an `asize` of 8 due to the previous print,
+	 * `ENSURE_SIZE` would not try to reallocate the array at
+	 * all. As it didn't explicitly check for `git_buf__oom`
+	 * in earlier versions, this would've resulted in it
+	 * returning successfully and thus `git_buf_puts` would
+	 * just print into the `git_buf__oom` array.
+	 */
+	cl_git_fail(git_buf_puts(&buf, "foobar"));
+}