Commit 95fa38802f12b930b3acf3fe842408bb33eb1d18

Patrick Steinhardt 2016-10-28T16:07:40

pqueue: resolve possible NULL pointer dereference The `git_pqueue` struct allows being fixed in its total number of entries. In this case, we simply throw away items that are inserted into the priority queue by examining wether the new item to be inserted has a higher priority than the previous smallest one. This feature somewhat contradicts our pqueue implementation in that it is allowed to not have a comparison function. In fact, we also fail to check if the comparison function is actually set in the case where we add a new item into a fully filled fixed-size pqueue. As we cannot determine which item is the smallest item in absence of a comparison function, we fix the `NULL` pointer dereference by simply dropping all new items which are about to be inserted into a full fixed-size pqueue.

diff --git a/src/pqueue.c b/src/pqueue.c
index 8cfc439..9341d1a 100644
--- a/src/pqueue.c
+++ b/src/pqueue.c
@@ -86,8 +86,9 @@ int git_pqueue_insert(git_pqueue *pq, void *item)
 	if ((pq->flags & GIT_PQUEUE_FIXED_SIZE) != 0 &&
 		pq->length >= pq->_alloc_size)
 	{
-		/* skip this item if below min item in heap */
-		if (pq->_cmp(item, git_vector_get(pq, 0)) <= 0)
+		/* skip this item if below min item in heap or if
+		 * we do not have a comparison function */
+		if (!pq->_cmp || pq->_cmp(item, git_vector_get(pq, 0)) <= 0)
 			return 0;
 		/* otherwise remove the min item before inserting new */
 		(void)git_pqueue_pop(pq);
diff --git a/tests/core/pqueue.c b/tests/core/pqueue.c
index bcd4eea..2b90f41 100644
--- a/tests/core/pqueue.c
+++ b/tests/core/pqueue.c
@@ -93,7 +93,29 @@ void test_core_pqueue__max_heap_size(void)
 	cl_assert_equal_i(0, git_pqueue_size(&pq));
 
 	git_pqueue_free(&pq);
+}
+
+void test_core_pqueue__max_heap_size_without_comparison(void)
+{
+	git_pqueue pq;
+	int i, vals[100] = { 0 };
+
+	cl_git_pass(git_pqueue_init(&pq, GIT_PQUEUE_FIXED_SIZE, 50, NULL));
+
+	for (i = 0; i < 100; ++i)
+		cl_git_pass(git_pqueue_insert(&pq, &vals[i]));
 
+	cl_assert_equal_i(50, git_pqueue_size(&pq));
+
+	/* As we have no comparison function, we cannot make any
+	 * actual assumptions about which entries are part of the
+	 * pqueue */
+	for (i = 0; i < 50; ++i)
+		cl_assert(git_pqueue_pop(&pq));
+
+	cl_assert_equal_i(0, git_pqueue_size(&pq));
+
+	git_pqueue_free(&pq);
 }
 
 static int cmp_ints_like_commit_time(const void *a, const void *b)