Skip to content

Commit 578fc29

Browse files
committed
next_branches(): Fix bugs leading to READ on freed shared memory
This patch fixes two code paths leading to the @avp pointer being freed, after which the dangling pointer is read afterwards by the search_next_avp() function at the "done" goto label. This will work 99% of the time, until the 1% where it won't (crash and burn!). Many thanks to Richard Revels (@rrevels-bw) and Sebastien Couture for an accurate report, as well as their involvement in troubleshooting! Fixes #2446 Fixes #2950
1 parent 83281ca commit 578fc29

File tree

1 file changed

+12
-15
lines changed

1 file changed

+12
-15
lines changed

serialize.c

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ int next_branches( struct sip_msg *msg)
281281
qvalue_t q;
282282
str uri, dst_uri, path, path_dst;
283283
char *p;
284-
unsigned int flags;
284+
unsigned int flags, last_parallel_fork;
285285
int rval;
286286

287287
if (route_type != REQUEST_ROUTE && route_type != FAILURE_ROUTE ) {
@@ -355,16 +355,14 @@ int next_branches( struct sip_msg *msg)
355355
path.len, path.s,
356356
q, flags, avp->flags);
357357

358-
359-
if (avp->flags & Q_FLAG) {
360-
destroy_avp(avp);
361-
goto done;
362-
}
363-
358+
last_parallel_fork = (avp->flags & Q_FLAG);
364359
prev = avp;
365-
avp = search_next_avp(prev, &val);
360+
avp = search_next_avp(avp, &val);
366361
destroy_avp(prev);
367362

363+
if (last_parallel_fork)
364+
goto done;
365+
368366
/* Append branches until out of branches or Q_FLAG is set */
369367
while (avp != NULL) {
370368

@@ -404,19 +402,18 @@ int next_branches( struct sip_msg *msg)
404402
goto error1;
405403
}
406404

407-
if (avp->flags & Q_FLAG) {
408-
destroy_avp(avp);
409-
goto done;
410-
}
411-
405+
last_parallel_fork = (avp->flags & Q_FLAG);
412406
prev = avp;
413-
avp = search_next_avp(prev, &val);
407+
avp = search_next_avp(avp, &val);
414408
destroy_avp(prev);
409+
410+
if (last_parallel_fork)
411+
goto done;
415412
}
416413

417414
return 2;
418415
done:
419-
return (search_next_avp(avp, NULL)==NULL)?2:1;
416+
return avp ? 1 : 2;
420417
error1:
421418
destroy_avp(avp);
422419
error:

0 commit comments

Comments
 (0)