From: Andre Noll <maan@tuebingen.mpg.de>
Date: Mon, 24 Jul 2017 15:26:44 +0000 (+0200)
Subject: vss: Avoid use after free in vss_send().
X-Git-Tag: v0.5.8~5
X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=bd33d7a13fa2cb65d37e5bbf5abdbb62e9b10610

vss: Avoid use after free in vss_send().

In vss_send() we increment the current slice number for the fec
client after fc->send_fec() has sent a slice. This results in a use
after free in case of a write error because ->send_fec() frees the
fec client structure on write errors. Valgrind complains about this
with the splat below.

To avoid this, the fc pointer must not be dereferenced after
->send_fec() has been called. This patch increases the current slice
number *before* the call to ->send_fec(). This works because the fec
clients do not care about this number.

The bug was introduced eight years ago in commit 625c5cd (Add forward
error correction code to the udp sender/receiver).

	==8615== Invalid read of size 1
	==8615==    at 0x805022B: vss_send (vss.c:1051)
	==8615==    by 0x805022B: vss_post_select (vss.c:1168)
	==8615==    by 0x8061DC7: call_post_select (sched.c:84)
	==8615==    by 0x8061DC7: sched_post_select (sched.c:110)
	==8615==    by 0x8061DC7: schedule (sched.c:163)
	==8615==    by 0x804CBFD: main (server.c:607)
	==8615==  Address 0x4670168 is 80 bytes inside a block of size 116 free'd
	==8615==    at 0x402D221: free (vg_replace_malloc.c:530)
	==8615==    by 0x8062D7C: udp_delete_target (udp_send.c:80)
	==8615==    by 0x80630DC: udp_send_fec (udp_send.c:305)
	==8615==    by 0x805022A: vss_send (vss.c:1049)
	==8615==    by 0x805022A: vss_post_select (vss.c:1168)
	==8615==    by 0x8061DC7: call_post_select (sched.c:84)
	==8615==    by 0x8061DC7: sched_post_select (sched.c:110)
	==8615==    by 0x8061DC7: schedule (sched.c:163)
	==8615==    by 0x804CBFD: main (server.c:607)
	==8615==  Block was alloc'd at
	==8615==    at 0x402C1F0: malloc (vg_replace_malloc.c:299)
	==8615==    by 0x8052D7E: para_malloc (string.c:67)
	==8615==    by 0x8052FBD: para_calloc (string.c:90)
	==8615==    by 0x804F48F: vss_add_fec_client (vss.c:686)
	==8615==    by 0x8063433: udp_com_add (udp_send.c:342)
	==8615==    by 0x8063688: udp_init_target_list (udp_send.c:395)
	==8615==    by 0x806371D: udp_send_init (udp_send.c:442)
	==8615==    by 0x805062A: init_vss_task (vss.c:1195)
	==8615==    by 0x804CA57: server_init (server.c:537)
	==8615==    by 0x804CA57: main (server.c:605)
---

diff --git a/vss.c b/vss.c
index 5484db9d..4d73a95c 100644
--- a/vss.c
+++ b/vss.c
@@ -1029,9 +1029,9 @@ static void vss_send(struct vss_task *vsst)
 			continue;
 		PARA_DEBUG_LOG("sending %u:%u (%u bytes)\n", fc->group.num,
 			fc->current_slice_num, fc->group.slice_bytes);
+		fc->current_slice_num++;
 		fc->fcp->send_fec(fc->sc, (char *)fc->enc_buf,
 			fc->group.slice_bytes + FEC_HEADER_SIZE);
-		fc->current_slice_num++;
 		fec_active = 1;
 	}
 	if (mmd->current_chunk >= mmd->afd.afhi.chunks_total) { /* eof */