From 26b8751d61fc92a2ba9f3878644c1a6fb1c4b673 Mon Sep 17 00:00:00 2001
From: f4exb <f4exb06@gmail.com>
Date: Tue, 5 Mar 2019 11:26:38 +0100
Subject: [PATCH] libfreedv: fixed possible out of bounds condition in
 freedv_data_channel_rx_frame

---
 libfreedv/fdmdv.cpp               |  2 +-
 libfreedv/freedv_data_channel.cpp | 81 ++++++++++++++++++++-----------
 2 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/libfreedv/fdmdv.cpp b/libfreedv/fdmdv.cpp
index c8094b3d7..efe38b4b9 100644
--- a/libfreedv/fdmdv.cpp
+++ b/libfreedv/fdmdv.cpp
@@ -96,7 +96,7 @@ struct FDMDV * fdmdv_create(int Nc)
     assert(f->rx_test_bits_mem != NULL);
     for(i=0; i<f->ntest_bits; i++)
 	f->rx_test_bits_mem[i] = 0;
-    assert((sizeof(test_bits)/sizeof(int)) >= f->ntest_bits);
+    assert((sizeof(test_bits)/sizeof(int)) >= (std::size_t) f->ntest_bits);
 
     f->old_qpsk_mapping = 0;
 
diff --git a/libfreedv/freedv_data_channel.cpp b/libfreedv/freedv_data_channel.cpp
index 8b51440f8..e29aca194 100644
--- a/libfreedv/freedv_data_channel.cpp
+++ b/libfreedv/freedv_data_channel.cpp
@@ -148,6 +148,7 @@ void freedv_data_set_cb_tx(struct freedv_data_channel *fdc, freedv_data_callback
 void freedv_data_channel_rx_frame(struct freedv_data_channel *fdc, unsigned char *data, std::size_t size, int from_bit, int bcast_bit, int crc_bit, int end_bits)
 {
     int copy_bits;
+
     if (end_bits) {
         copy_bits = end_bits;
     } else {
@@ -155,76 +156,102 @@ void freedv_data_channel_rx_frame(struct freedv_data_channel *fdc, unsigned char
     }
 
     /* New packet? */
-    if (fdc->packet_rx_cnt == 0) {
+    if (fdc->packet_rx_cnt == 0)
+    {
         /* Does the packet have a compressed from field? */
         if (from_bit) {
-	    /* Compressed from: take the previously received header */
-	    memcpy(fdc->packet_rx + fdc->packet_rx_cnt, fdc->rx_header, 6);
-	    fdc->packet_rx_cnt += 6;
-       	}
-	if (bcast_bit) {
-            if (!from_bit) {
+            /* Compressed from: take the previously received header */
+            memcpy(fdc->packet_rx + fdc->packet_rx_cnt, fdc->rx_header, 6);
+            fdc->packet_rx_cnt += 6;
+        }
+
+        if (bcast_bit)
+        {
+            if (!from_bit)
+            {
                 /* Copy from header and modify size and end_bits accordingly */
                 memcpy(fdc->packet_rx + fdc->packet_rx_cnt, data, 6);
                 fdc->packet_rx_cnt += 6;
                 copy_bits -= 6;
-                if (copy_bits < 0)
+
+                if (copy_bits < 0) {
                     copy_bits = 0;
+                }
+
                 data += 6;
             }
             /* Compressed to: fill in broadcast address */
             memcpy(fdc->packet_rx + fdc->packet_rx_cnt, fdc_header_bcast, sizeof(fdc_header_bcast));
             fdc->packet_rx_cnt += 6;
-	}
-        if (crc_bit) {
+        }
+
+        if (crc_bit)
+        {
             unsigned char calc_crc = fdc_crc4(data, size);
-            if (calc_crc == end_bits) {
-	        /* It is a single header field, remember it for later */
+
+            if (calc_crc == end_bits)
+            {
+                /* It is a single header field, remember it for later */
                 memcpy(fdc->packet_rx + 6, data, 6);
-		memcpy(fdc->packet_rx, fdc_header_bcast, 6);
+                memcpy(fdc->packet_rx, fdc_header_bcast, 6);
+
                 if (fdc->cb_rx) {
                     fdc->cb_rx(fdc->cb_rx_state, fdc->packet_rx, 12);
                 }
             }
+
             fdc->packet_rx_cnt = 0;
             return;
         }
     }
 
-    if (fdc->packet_rx_cnt + copy_bits >= FREEDV_DATA_CHANNEL_PACKET_MAX) {
-        /* Something went wrong... this can not be a real packet */
-	fdc->packet_rx_cnt = 0;
-	return;
+    if (fdc->packet_rx_cnt + copy_bits >= FREEDV_DATA_CHANNEL_PACKET_MAX)
+    {
+        // Something went wrong... this can not be a real packet
+        fdc->packet_rx_cnt = 0;
+        return;
+    }
+    else if (fdc->packet_rx_cnt < 0)
+    {
+        // This is wrong too...
+        fdc->packet_rx_cnt = 0;
+        return;
     }
 
     memcpy(fdc->packet_rx + fdc->packet_rx_cnt, data, copy_bits);
     fdc->packet_rx_cnt += copy_bits;
 
-    if (end_bits != 0 && fdc->packet_rx_cnt >= 2) {
+    if (end_bits != 0 && fdc->packet_rx_cnt >= 2)
+    {
         unsigned short calc_crc = fdc_crc(fdc->packet_rx, fdc->packet_rx_cnt - 2);
         unsigned short rx_crc;
-	rx_crc = fdc->packet_rx[fdc->packet_rx_cnt - 1] << 8;
+        rx_crc = fdc->packet_rx[fdc->packet_rx_cnt - 1] << 8;
         rx_crc |= fdc->packet_rx[fdc->packet_rx_cnt - 2];
 
-        if (rx_crc == calc_crc) {
+        if (rx_crc == calc_crc)
+        {
             if ((std::size_t) fdc->packet_rx_cnt == size) {
-	        /* It is a single header field, remember it for later */
+                /* It is a single header field, remember it for later */
                 memcpy(fdc->rx_header, fdc->packet_rx, 6);
             }
 
             /* callback */
-            if (fdc->cb_rx) {
+            if (fdc->cb_rx)
+            {
                 unsigned char tmp[6];
-		memcpy(tmp, fdc->packet_rx, 6);
-		memcpy(fdc->packet_rx, fdc->packet_rx + 6, 6);
-		memcpy(fdc->packet_rx + 6, tmp, 6);
+                memcpy(tmp, fdc->packet_rx, 6);
+                memcpy(fdc->packet_rx, fdc->packet_rx + 6, 6);
+                memcpy(fdc->packet_rx + 6, tmp, 6);
+                std::size_t size = fdc->packet_rx_cnt - 2;
 
-		std::size_t size = fdc->packet_rx_cnt - 2;
-                if (size < 12)
+                if (size < 12) {
                     size = 12;
+                }
+
                 fdc->cb_rx(fdc->cb_rx_state, fdc->packet_rx, size);
             }
         }
+
         fdc->packet_rx_cnt = 0;
     }
 }