librelist archives

« back to archive

[PATCH 1/2] Fixed two buffer handling errors in vector.c

[PATCH 1/2] Fixed two buffer handling errors in vector.c

From:
Alex Budovski
Date:
2011-01-07 @ 01:26
- remove() would read one-past array bounds.
- resize() would fail if the initial size was 1, because it multiplied by 1.75
  and truncated the resulting value. The buffer would always remain at size 1,
  but elements would repeatedly be appended (via insert()) causing a crash.
---
 src/vector.c         |    6 ++++--
 tests/t0004-vector.c |   27 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 tests/t0004-vector.c

diff --git a/src/vector.c b/src/vector.c
index 47e8ce8..3ca594a 100644
--- a/src/vector.c
+++ b/src/vector.c
@@ -27,6 +27,8 @@
 #include "repository.h"
 #include "vector.h"
 
+#include <math.h>
+
 static const double resize_factor = 1.75;
 static const int minimum_size = 8;
 
@@ -34,7 +36,7 @@ static int resize_vector(git_vector *v)
 {
 	void **new_contents;
 
-	v->_alloc_size = (unsigned int)(v->_alloc_size * resize_factor);
+	v->_alloc_size = (unsigned int)ceil(v->_alloc_size * resize_factor);
 	if (v->_alloc_size == 0)
 		v->_alloc_size = minimum_size;
 
@@ -130,7 +132,7 @@ int git_vector_remove(git_vector *v, unsigned int idx)
 	if (idx >= v->length || v->length == 0)
 		return GIT_ENOTFOUND;
 
-	for (i = idx; i < v->length; ++i)
+	for (i = idx; i < v->length - 1; ++i)
 		v->contents[i] = v->contents[i + 1];
 
 	v->length--;
diff --git a/tests/t0004-vector.c b/tests/t0004-vector.c
new file mode 100644
index 0000000..bee71d2
--- /dev/null
+++ b/tests/t0004-vector.c
@@ -0,0 +1,27 @@
+#include "test_lib.h"
+#include "common.h"
+#include "vector.h"
+
+/* Initial size of 1 will cause writing past array bounds prior to fix */
+BEGIN_TEST(initial_size_one)
+  git_vector x;
+  int i;
+  git_vector_init(&x, 1, NULL, NULL);
+  for (i = 0; i < 10; ++i) {
+    git_vector_insert(&x, (void*) 0xabc);
+  }
+  git_vector_free(&x);
+END_TEST
+
+/* vector used to read past array bounds on remove() */
+BEGIN_TEST(remove)
+  git_vector x;
+  // make initial capacity exact for our insertions.
+  git_vector_init(&x, 3, NULL, NULL);
+  git_vector_insert(&x, (void*) 0xabc);
+  git_vector_insert(&x, (void*) 0xdef);
+  git_vector_insert(&x, (void*) 0x123);
+
+  git_vector_remove(&x, 0);  // used to read past array bounds.
+  git_vector_free(&x);
+END_TEST
-- 
1.7.3.1.msysgit.0

[PATCH 2/2] Revised build configuration for MSVC.

From:
Alex Budovski
Date:
2011-01-07 @ 01:26
Major changes and rationale:
- /WX: absolutely vital when compiling in C-mode as the compiler is
  incredibly lenient on what is allowed to compile. It allows functions to be
  called without prototypes declared, treating them as functions returning int
  taking an unspecified (read: unrestricted) list of arguments, without any
  type checking! It will simply issue a warning, which is easily overlooked.

  A real example: it will allow you to call ceil(1.75) without first including
  <math.h> causing UB, returning bogus results like 1023 on the machine I
  tested on.

- Release build separate from debug.
  Presently release builds don't exist.  Consequently they are completely
  untested. Many bugs may only manifest themselves in release mode. The current
  configuration sets debug-only flags like /RTC1 which are incompatible with
  optimization (/O2).

  In addition, the Windows build of libgit2 has no optimized version. This
  change resolves this.

- Added checksum generation in image headers. This is so debuggers don't
  complain about checksum mismatches and provides a small amount of consistency
  to binaries.
---
 wscript |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/wscript b/wscript
index d6f3508..c2311de 100644
--- a/wscript
+++ b/wscript
@@ -2,11 +2,18 @@ from waflib.Context import Context
 from waflib.Build import BuildContext, CleanContext, \
         InstallContext, UninstallContext
 
+# Unix flags
 CFLAGS_UNIX = ["-O2", "-Wall", "-Wextra"]
-CFLAGS_WIN32 = ['/TC', '/W4', '/RTC1', '/nologo']
-
 CFLAGS_UNIX_DBG = ['-g']
-CFLAGS_WIN32_DBG = ['/Zi', '/DEBUG']
+
+# Windows MSVC flags
+CFLAGS_WIN32_COMMON = ['/TC', '/W4', '/WX', '/nologo', '/Zi']
+CFLAGS_WIN32_RELEASE = ['/O2', '/MD']
+
+# Note: /RTC* cannot be used with optimization on.
+CFLAGS_WIN32_DBG = ['/Od', '/RTC1', '/RTCc', '/DEBUG', '/MDd']
+CFLAGS_WIN32_L = ['/RELEASE']  # used for /both/ debug and release builds.
+                               # sets the module's checksum in the header.
 CFLAGS_WIN32_L_DBG = ['/DEBUG']
 
 ALL_LIBS = ['z', 'crypto', 'pthread']
@@ -43,8 +50,10 @@ def configure(conf):
         conf.env.PLATFORM = 'win32'
 
         if conf.env.CC_NAME == 'msvc':
-            conf.env.CFLAGS = CFLAGS_WIN32 + (CFLAGS_WIN32_DBG if dbg else [])
-            conf.env.LINKFLAGS += CFLAGS_WIN32_L_DBG if dbg else []
+            conf.env.CFLAGS = CFLAGS_WIN32_COMMON + \
+              (CFLAGS_WIN32_DBG if dbg else CFLAGS_WIN32_RELEASE)
+            conf.env.LINKFLAGS += CFLAGS_WIN32_L + \
+              (CFLAGS_WIN32_L_DBG if dbg else [])
             conf.env.DEFINES += ['WIN32', '_DEBUG', '_LIB', 'ZLIB_WINAPI']
             zlib_name = 'zlibwapi'
 
-- 
1.7.3.1.msysgit.0