mirror of
https://git.proxmox.com/git/mirror_frr
synced 2025-08-03 20:55:25 +00:00
lib: graph: fix vector_remove()
vector_remove would corrupt the data in the following sequence: 1. assume vector v = [a, b], active = 2 2. vector_unset(v, 0) => v = [NULL, b], active = 2 3. vector_remove(v, 1) vector_remove calls vector_unset(v, 1), vector_unset notices index #0 is also NULL and thus sets active to 0. The equality test in vector_remove() now fails, leading it to decrement v->active *again*, leading to an underflow that will likely crash the daemon (and might even be exploitable). This call sequence does not happen in existing code since vector_unset() is not used on graph from/to lists. Nonetheless this is a buried land mine in the code at best. Rewrite the function - while we're at it, there's no reason to move the entire array around, just fill the hole with the last element. Signed-off-by: David Lamparter <equinox@opensourcerouting.org> Cc: Quentin Young <qlyoung@cumulusnetworks.com>
This commit is contained in:
parent
5bf313994d
commit
c30420200b
13
lib/graph.c
13
lib/graph.c
@ -55,11 +55,16 @@ graph_new_node (struct graph *graph, void *data, void (*del) (void*))
|
|||||||
static void
|
static void
|
||||||
vector_remove (vector v, unsigned int ix)
|
vector_remove (vector v, unsigned int ix)
|
||||||
{
|
{
|
||||||
vector_unset (v, ix);
|
if (ix >= v->active)
|
||||||
if (ix == vector_active (v)) return;
|
return;
|
||||||
for (; ix < vector_active (v) - 1; ix++)
|
|
||||||
v->index[ix] = v->index[ix+1];
|
/* v->active is guaranteed >= 1 because ix can't be lower than 0
|
||||||
|
* and v->active is > ix. */
|
||||||
v->active--;
|
v->active--;
|
||||||
|
/* if ix == v->active--, we set the item to itself, then to NULL...
|
||||||
|
* still correct, no check neccessary. */
|
||||||
|
v->index[ix] = v->index[v->active];
|
||||||
|
v->index[v->active] = NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
void
|
void
|
||||||
|
Loading…
Reference in New Issue
Block a user