Browse code

core: tcp - more safety check to avoid jumping out if receive buf for ws proto

Daniel-Constantin Mierla authored on 07/09/2018 06:22:53
Showing 1 changed files
... ...
@@ -241,16 +241,17 @@ int tcp_read_data(int fd, struct tcp_connection *c,
241 241
 					char* buf, int b_size, int* flags)
242 242
 {
243 243
 	int bytes_read;
244
-	
244
+
245 245
 again:
246 246
 	bytes_read=read(fd, buf, b_size);
247
-	
247
+
248 248
 	if (likely(bytes_read!=b_size)){
249 249
 		if(unlikely(bytes_read==-1)){
250 250
 			if (errno == EWOULDBLOCK || errno == EAGAIN){
251 251
 				bytes_read=0; /* nothing has been read */
252
-			}else if (errno == EINTR) goto again;
253
-			else{
252
+			}else if (errno == EINTR){
253
+				goto again;
254
+			}else{
254 255
 				if (unlikely(c->state==S_CONN_CONNECT)){
255 256
 					switch(errno){
256 257
 						case ECONNRESET:
... ...
@@ -299,7 +300,8 @@ again:
299 300
 						"error reading: %s (%d) ([%s]:%u ->",
300 301
 						strerror(errno), errno,
301 302
 						ip_addr2a(&c->rcv.src_ip), c->rcv.src_port);
302
-				LOG(cfg_get(core, core_cfg, corelog),"-> [%s]:%u)\n", ip_addr2a(&c->rcv.dst_ip), c->rcv.dst_port);
303
+				LOG(cfg_get(core, core_cfg, corelog),"-> [%s]:%u)\n",
304
+						ip_addr2a(&c->rcv.dst_ip), c->rcv.dst_port);
303 305
 				if (errno == ETIMEDOUT) {
304 306
 					tcp_emit_closed_event(c, TCP_CLOSED_TIMEOUT);
305 307
 				} else if (errno == ECONNRESET) {
... ...
@@ -307,12 +309,13 @@ again:
307 309
 				}
308 310
 				return -1;
309 311
 			}
310
-		}else if (unlikely((bytes_read==0) || 
312
+		}else if (unlikely((bytes_read==0) ||
311 313
 					(*flags & RD_CONN_FORCE_EOF))){
312 314
 			c->state=S_CONN_EOF;
313 315
 			*flags|=RD_CONN_EOF;
314 316
 			tcp_emit_closed_event(c, TCP_CLOSED_EOF);
315
-			LM_DBG("EOF on %p, FD %d ([%s]:%u ->", c, fd, ip_addr2a(&c->rcv.src_ip), c->rcv.src_port);
317
+			LM_DBG("EOF on %p, FD %d ([%s]:%u ->", c, fd,
318
+					ip_addr2a(&c->rcv.src_ip), c->rcv.src_port);
316 319
 			LM_DBG("-> [%s]:%u)\n", ip_addr2a(&c->rcv.dst_ip), c->rcv.dst_port);
317 320
 		}else{
318 321
 			if (unlikely(c->state==S_CONN_CONNECT || c->state==S_CONN_ACCEPT)){
... ...
@@ -336,9 +339,9 @@ again:
336 339
 /* reads next available bytes
337 340
  *   c- tcp connection used for reading, tcp_read changes also c->state on
338 341
  *      EOF and c->req.error on read error
339
- *   * flags - value/result - used to signal a seen or "forced" EOF on the 
340
- *     connection (when it is known that no more data will come after the 
341
- *     current socket buffer is emptied )=> return/signal EOF on the first 
342
+ *   * flags - value/result - used to signal a seen or "forced" EOF on the
343
+ *     connection (when it is known that no more data will come after the
344
+ *     current socket buffer is emptied )=> return/signal EOF on the first
342 345
  *     short read (=> don't use it on POLLPRI, as OOB data will cause short
343 346
  *      reads even if there are still remaining bytes in the socket buffer)
344 347
  * return number of bytes read, 0 on EOF or -1 on error,
... ...
@@ -354,8 +357,8 @@ int tcp_read(struct tcp_connection *c, int* flags)
354 357
 
355 358
 	r=&c->req;
356 359
 	fd=c->fd;
357
-	bytes_free=r->b_size- (int)(r->pos - r->buf);
358
-	
360
+	bytes_free=r->b_size - (unsigned int)(r->pos - r->buf);
361
+
359 362
 	if (unlikely(bytes_free<=0)){
360 363
 		LM_ERR("buffer overrun, dropping ([%s]:%u -> [%s]:%u)\n",
361 364
 				ip_addr2a(&c->rcv.src_ip), c->rcv.src_port,
... ...
@@ -1114,10 +1117,28 @@ static int tcp_read_ws(struct tcp_connection *c, int* read_flags)
1114 1117
 #endif
1115 1118
 		bytes = tcp_read(c, read_flags);
1116 1119
 
1117
-	if (bytes <= 0)
1118
-	{
1119
-		if (likely(r->parsed >= r->pos))
1120
-			return 0;
1120
+	if (bytes < 0) {
1121
+		/* read error */
1122
+		return bytes;
1123
+	}
1124
+	if (r->parsed == r->pos) {
1125
+		/* nothing else to parse */
1126
+		return bytes;
1127
+	}
1128
+	if (r->parsed > r->pos) {
1129
+		LM_ERR("req buf pos (%p) before parsed (%p) [%d]\n", r->pos, r->parsed,
1130
+				bytes);
1131
+		return -1;
1132
+	}
1133
+	if(r->pos > r->buf + r->b_size) {
1134
+		LM_ERR("req pos (%p) over buf (%p / %u) - parsed (%p) [%d]\n", r->pos,
1135
+				r->buf, r->b_size, r->parsed, bytes);
1136
+		return -1;
1137
+	}
1138
+	if(r->buf > r->parsed) {
1139
+		LM_ERR("req parsed (%p) before buf (%p / %u) - pos (%p) [%d]\n",
1140
+				r->parsed, r->buf, r->b_size, r->pos, bytes);
1141
+		return -1;
1121 1142
 	}
1122 1143
 
1123 1144
 	size = r->pos - r->parsed;
... ...
@@ -1154,11 +1175,11 @@ static int tcp_read_ws(struct tcp_connection *c, int* read_flags)
1154 1175
 		goto skip;
1155 1176
 	pos++;
1156 1177
 	mask_present = p[pos] & 0x80;
1157
-	len = (p[pos++] & 0xff) & ~0x80;
1178
+	len = (p[pos] & 0xff) & ~0x80;
1179
+	pos++;
1158 1180
 
1159 1181
 	/* Work out real length */
1160
-	if (len == 126)
1161
-	{
1182
+	if (len == 126) {
1162 1183
 		/* 2 bytes store the payload size */
1163 1184
 		if (size < pos + 2)
1164 1185
 			goto skip;
... ...
@@ -1187,8 +1208,7 @@ static int tcp_read_ws(struct tcp_connection *c, int* read_flags)
1187 1208
 	}
1188 1209
 
1189 1210
 	/* Skip mask */
1190
-	if (mask_present)
1191
-	{
1211
+	if (mask_present) {
1192 1212
 		if (size < pos + 4)
1193 1213
 			goto skip;
1194 1214
 		pos += 4;
... ...
@@ -1452,10 +1472,10 @@ int tcp_read_req(struct tcp_connection* con, int* bytes_read, int* read_flags)
1452 1472
 	char c;
1453 1473
 	int ret;
1454 1474
 
1455
-		bytes=-1;
1456
-		total_bytes=0;
1457
-		resp=CONN_RELEASE;
1458
-		req=&con->req;
1475
+	bytes=-1;
1476
+	total_bytes=0;
1477
+	resp=CONN_RELEASE;
1478
+	req=&con->req;
1459 1479
 
1460 1480
 again:
1461 1481
 		if (likely(req->error==TCP_REQ_OK)){
... ...
@@ -1479,10 +1499,10 @@ again:
1479 1499
 			}
1480 1500
 #endif
1481 1501
 
1482
-			if (unlikely(bytes==-1)){
1502
+			if (unlikely(bytes<0)){
1483 1503
 				LOG(cfg_get(core, core_cfg, corelog),
1484
-						"ERROR: tcp_read_req: error reading - c: %p r: %p\n",
1485
-						con, req);
1504
+						"ERROR: tcp_read_req: error reading - c: %p r: %p (%d)\n",
1505
+						con, req, bytes);
1486 1506
 				resp=CONN_ERROR;
1487 1507
 				goto end_req;
1488 1508
 			}
... ...
@@ -1610,14 +1630,14 @@ again:
1610 1630
 #endif
1611 1631
 				ret = receive_tcp_msg(req->start, req->parsed-req->start,
1612 1632
 									&con->rcv, con);
1613
-				
1633
+
1614 1634
 			if (unlikely(ret < 0)) {
1615 1635
 				*req->parsed=c;
1616 1636
 				resp=CONN_ERROR;
1617 1637
 				goto end_req;
1618 1638
 			}
1619 1639
 			*req->parsed=c;
1620
-			
1640
+
1621 1641
 			/* prepare for next request */
1622 1642
 			size=req->pos-req->parsed;
1623 1643
 			req->start=req->buf;
... ...
@@ -1628,8 +1648,8 @@ again:
1628 1648
 			req->content_len=0;
1629 1649
 			req->bytes_to_go=0;
1630 1650
 			req->pos=req->buf+size;
1631
-			
1632
-			if (unlikely(size)){ 
1651
+
1652
+			if (unlikely(size)){
1633 1653
 				memmove(req->buf, req->parsed, size);
1634 1654
 				req->parsed=req->buf; /* fix req->parsed after using it */
1635 1655
 #ifdef EXTRA_DEBUG
... ...
@@ -1645,8 +1665,7 @@ again:
1645 1665
 			}
1646 1666
 			req->parsed=req->buf; /* fix req->parsed */
1647 1667
 		}
1648
-		
1649
-		
1668
+
1650 1669
 	end_req:
1651 1670
 		if (likely(bytes_read)) *bytes_read=total_bytes;
1652 1671
 		return resp;