From fb8a64656150ba246a10f41e905490479c73b370 Mon Sep 17 00:00:00 2001
From: Jonathan Hanks <jonathan.hanks@ligo.org>
Date: Mon, 12 Oct 2020 17:25:09 -0700
Subject: [PATCH] Work on #186 parameter sanitation for daqd and transports.

This does some more checks in the daqd producer to verify that dcu id/counts are sane as well as there being no overflows.

Updates to dix_xmit and to the daq_core files to make total models and fulldatablocksize unsigned.  This allows a check for size to skip if the send length is < 0 before writing to dolphin.
---
 src/daqd/producer_shmem.cc | 42 +++++++++++++++++++++++++++++++-------
 src/include/daq_core.h     |  8 ++++----
 src/ix_stream/dix_xmit.c   | 42 +++++++++++++++++++-------------------
 3 files changed, 60 insertions(+), 32 deletions(-)

diff --git a/src/daqd/producer_shmem.cc b/src/daqd/producer_shmem.cc
index 75494ef41..b40929914 100644
--- a/src/daqd/producer_shmem.cc
+++ b/src/daqd/producer_shmem.cc
@@ -431,21 +431,49 @@ producer::frame_writer( )
         // map out the order of the dcuids in the zmq data, this could change
         // with each data block
         {
-            int   total_zmq_models = data_block->header.dcuTotalModels;
+            int total_zmq_models = data_block->header.dcuTotalModels;
+            if ( total_zmq_models >= DAQ_TRANSIT_MAX_DCU )
+            {
+                fprintf( stderr,
+                         "Too many dcus %d, this looks like corrupt data, "
+                         "aborting\n",
+                         total_zmq_models );
+                exit( 1 );
+            }
             char* cur_dcu_zmq_ptr = data_block->dataBlock;
+            char* end_data = cur_dcu_zmq_ptr + sizeof( data_block->dataBlock );
+            static_assert(
+                sizeof( data_block->dataBlock ) > sizeof( char* ),
+                "Make sure we are referring to the whole data block in a "
+                "daq_dc_data_t, not just a pointer to the data" );
             for ( int cur_block = 0; cur_block < total_zmq_models; ++cur_block )
             {
                 unsigned int cur_dcuid =
                     data_block->header.dcuheader[ cur_block ].dcuId;
-                dcu_to_zmq_lookup[ cur_dcuid ] = cur_block;
-                dcu_data_from_zmq[ cur_dcuid ] = cur_dcu_zmq_ptr;
-                dcu_data_crc[ cur_dcuid ] =
-                    data_block->header.dcuheader[ cur_block ].dataCrc;
-                dcu_data_gps[ cur_dcuid ] =
-                    data_block->header.dcuheader[ cur_block ].timeSec;
+                if ( cur_dcuid >= DCU_COUNT )
+                {
+                    std::cerr << "Skipping a high dcu data block " << cur_dcuid
+                              << std::endl;
+                }
+                else
+                {
+                    dcu_to_zmq_lookup[ cur_dcuid ] = cur_block;
+                    dcu_data_from_zmq[ cur_dcuid ] = cur_dcu_zmq_ptr;
+                    dcu_data_crc[ cur_dcuid ] =
+                        data_block->header.dcuheader[ cur_block ].dataCrc;
+                    dcu_data_gps[ cur_dcuid ] =
+                        data_block->header.dcuheader[ cur_block ].timeSec;
+                }
                 cur_dcu_zmq_ptr +=
                     data_block->header.dcuheader[ cur_block ].dataBlockSize +
                     data_block->header.dcuheader[ cur_block ].tpBlockSize;
+                if ( cur_dcu_zmq_ptr > end_data )
+                {
+                    std::cerr
+                        << "The current datablock has overflowed, aborting"
+                        << std::endl;
+                    exit( 1 );
+                }
             }
         }
 
diff --git a/src/include/daq_core.h b/src/include/daq_core.h
index 3776246c4..84f2d5dc9 100644
--- a/src/include/daq_core.h
+++ b/src/include/daq_core.h
@@ -60,9 +60,9 @@ typedef struct daq_msg_header_t
 
 typedef struct daq_multi_dcu_header_t
 {
-    int dcuTotalModels; // Number of models
-    int fullDataBlockSize; // Number of bytes used in the data block (including
-                           // TP data)
+    unsigned int dcuTotalModels; // Number of models
+    unsigned int fullDataBlockSize; // Number of bytes used in the data block
+                                    // (including TP data)
     daq_msg_header_t dcuheader[ DAQ_TRANSIT_MAX_DCU ];
 } daq_multi_dcu_header_t;
 
@@ -87,7 +87,7 @@ typedef struct daq_multi_cycle_header_t
     unsigned int cycleDataSize; // stride in bytes of the data
     // max data size is assumed to be
     // at least maxCycle * cycleDataSize
-    int msgcrc; // Data CRC checksum for DC -> FB/NDS
+    unsigned int msgcrc; // Data CRC checksum for DC -> FB/NDS
 } daq_multi_cycle_header_t;
 
 // Data structure to support multiple cycles of multiple dcus
diff --git a/src/ix_stream/dix_xmit.c b/src/ix_stream/dix_xmit.c
index dd1f93fa0..e75d33232 100644
--- a/src/ix_stream/dix_xmit.c
+++ b/src/ix_stream/dix_xmit.c
@@ -317,27 +317,27 @@ main( int argc, char** argv )
                  (long)xmitDataOffset[ ii ] );
     }
 
-    int              nextCycle = 0;
-    int64_t          mytime = 0;
-    int64_t          mylasttime = 0;
-    int64_t          myptime = 0;
-    int64_t          n_cycle_time = 0;
-    int              mytotaldcu = 0;
-    char*            zbuffer;
-    size_t           zbuffer_remaining = 0;
-    int              dc_datablock_size = 0;
-    int              datablock_size_running = 0;
-    int              datablock_size_mb_s = 0;
-    static const int header_size = sizeof( daq_multi_dcu_header_t );
-    char             dcs[ 48 ];
-    int              edcuid[ 10 ];
-    int              estatus[ 10 ];
-    unsigned long    ets = 0;
-    int              timeout = 0;
-    int              threads_rdy;
-    int              any_rdy = 0;
-    int              jj, kk;
-    int              sendLength = 0;
+    int                       nextCycle = 0;
+    int64_t                   mytime = 0;
+    int64_t                   mylasttime = 0;
+    int64_t                   myptime = 0;
+    int64_t                   n_cycle_time = 0;
+    int                       mytotaldcu = 0;
+    char*                     zbuffer;
+    size_t                    zbuffer_remaining = 0;
+    int                       dc_datablock_size = 0;
+    int                       datablock_size_running = 0;
+    int                       datablock_size_mb_s = 0;
+    static const unsigned int header_size = sizeof( daq_multi_dcu_header_t );
+    char                      dcs[ 48 ];
+    int                       edcuid[ 10 ];
+    int                       estatus[ 10 ];
+    unsigned long             ets = 0;
+    int                       timeout = 0;
+    int                       threads_rdy;
+    int                       any_rdy = 0;
+    int                       jj, kk;
+    unsigned int              sendLength = 0;
 
     int     min_cycle_time = 1 << 30;
     int     pv_min_cycle_time = 0;
-- 
GitLab