Project

General

Profile

Bug #548

Read-past-end-of-buffer in libstrongswan/collections/array.c

Added by Noam Lampert over 6 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Normal
Category:
libstrongswan
Target version:
Start date:
18.03.2014
Due date:
Estimated time:
Affected version:
5.1.2
Resolution:
Fixed

Description

Below is a proposed patch.
If the array is new, and you remove the last element, memmove() will read past the end of the array.

static void remove_tail(array_t *array, int idx) {
/* move all items after idx one down */
memmove(array->data + get_size(array, idx + array->head),
array->data + get_size(array, idx + array->head + 1),
- get_size(array, array->count - idx));
+ get_size(array, array->count - (idx+1)));
array->count--;
array->tail++;
}

Associated revisions

Revision 11f31ceb (diff)
Added by Tobias Brunner over 6 years ago

array: Fix removal of elements in the second half of an array

Memory beyond the end of the array was moved when array elements in the
second half of an array were removed.

Fixes #548.

History

#1 Updated by Tobias Brunner over 6 years ago

  • Status changed from New to Feedback
  • Assignee set to Tobias Brunner

If the array is new, and you remove the last element, memmove() will read past the end of the array.

What do you mean? If it is new it is empty, so calling array_remove(array, ARRAY_TAIL, NULL) will return FALSE and does nothing else (definitely not call remove_tail()).

#2 Updated by Noam Lampert over 6 years ago

This test pretty much reproduces the error:
array_t* array = array_create(sizeof(int), 0);
int elem = 1;
for (int i = 0; i < 8; i++)
array_insert(array, i, &elem);
array_remove(array, 7, NULL);
array_destroy(array);

at array_remove(), remove_tail() is called. This illegally copies the array8 to array7.

#3 Updated by Tobias Brunner over 6 years ago

  • Tracker changed from Issue to Bug
  • Status changed from Feedback to Closed
  • Target version set to 5.1.3
  • Resolution set to Fixed

This test pretty much reproduces the error:

Thanks for the example code. I agree, this is incorrect. Fixed with the associated commit.

Also available in: Atom PDF