Out of bound Read found in TraceFmtDcdImpl::extractFrame()

[yabinc]

It's reported by kj2648@gmail.com in Android. Forward it to upstream:

Issue Description

Out of bound Read found in libopencsd_decoder due to bound check fail of dataPtr in TraceFmtDcdImpl::extractFrame() in trc_frame_deformatter.cpp.

It may cause security vulnerability by reading invalid memory address in memcpy.

external/OpenCSD/decoder/source/trc_frame_deformatter.cpp

(line 517, 526, 569) There are invalid bound checks. In blocks after cont_process bool checking condition,

(line 522, 536, 569) uint32_t or uint16_t dereference after invalid bound check causes OOB read.

459 bool TraceFmtDcdImpl::extractFrame()
460 {
...
514         const uint8_t *dataPtr = m_in_block_base+m_in_block_processed;
515         const uint8_t *eodPtr = m_in_block_base+m_in_block_size;
516
517         cont_process = (bool)(dataPtr < eodPtr);  <- bound check fail (1)
518
519         // can have FSYNCS at start of frame (in middle is an error).
520         if(hasFSyncs && cont_process && (m_ex_frm_n_bytes == 0))
521         {
522             while((*((uint32_t *)(dataPtr)) == FSYNC_PATTERN) && cont_process)  <- OOB Read! (1)
523             {
524                 f_sync_bytes += 4;
525                 dataPtr += 4;
526                 cont_process = (bool)(dataPtr < eodPtr);  <- bound check fail (2)
527             }
528         }
529
530         // not an FSYNC
531         while((m_ex_frm_n_bytes < OCSD_DFRMTR_FRAME_SIZE) && cont_process)
532         {
533             // check for illegal out of sequence FSYNC
534             if((m_ex_frm_n_bytes % 4) == 0)
535             {
536                 if(*((uint32_t *)(dataPtr)) == FSYNC_PATTERN)  <- OOB Read! (2)
537                 {
538                     // throw an illegal FSYNC error
539                     throw ocsdError(OCSD_ERR_SEV_ERROR, OCSD_ERR_DFMTR_BAD_FHSYNC, m_trc_curr_idx, "Bad FSYNC in frame.");
540                 }
541             }
542
543             // mark start of frame after FSyncs
544             if(m_ex_frm_n_bytes == 0)
545                 m_trc_curr_idx_sof = m_trc_curr_idx + f_sync_bytes;
546
547             m_ex_frm_data[m_ex_frm_n_bytes] = dataPtr[0];
548             m_ex_frm_data[m_ex_frm_n_bytes+1] = dataPtr[1];
549             m_ex_frm_n_bytes+=2;
550             ex_bytes +=2;
551
552             // check pair is not HSYNC
553             if(*((uint16_t *)(dataPtr)) == HSYNC_PATTERN)  <- OOB Read! (3)
554             {
555                 if(hasHSyncs)
556                 {
557                     m_ex_frm_n_bytes-=2;
558                     ex_bytes -= 2;
559                     h_sync_bytes+=2;
560                 }
561                 else
562                 {
563                     // throw illegal HSYNC error.
564                     throw ocsdError(OCSD_ERR_SEV_ERROR, OCSD_ERR_DFMTR_BAD_FHSYNC, m_trc_curr_idx, "Bad HSYNC in frame.");
565                 }
566             }
567
568             dataPtr += 2;
569             cont_process = (bool)(dataPtr < eodPtr);  <- bound check fail (3)
570         }
571
572         // if we hit the end of data but still have a complete frame waiting,
573         // need to continue processing to allow it to be used.
574         if(!cont_process && (m_ex_frm_n_bytes == OCSD_DFRMTR_FRAME_SIZE))
575             cont_process = true;
576     }

Suggest fix:

0002-Fix-Out-of-Bounds-in-TraceFmtDcdImpl-extractFrame.patch

---
 decoder/source/trc_frame_deformatter.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/decoder/source/trc_frame_deformatter.cpp b/decoder/source/trc_frame_deformatter.cpp
index dc12e3f..afa1d1f 100644
--- a/decoder/source/trc_frame_deformatter.cpp
+++ b/decoder/source/trc_frame_deformatter.cpp
@@ -514,7 +514,7 @@ bool TraceFmtDcdImpl::extractFrame()
         const uint8_t *dataPtr = m_in_block_base+m_in_block_processed;
         const uint8_t *eodPtr = m_in_block_base+m_in_block_size;

-        cont_process = (bool)(dataPtr < eodPtr);
+        cont_process = (bool)(dataPtr + sizeof(uint32_t) < eodPtr);

         // can have FSYNCS at start of frame (in middle is an error).
         if(hasFSyncs && cont_process && (m_ex_frm_n_bytes == 0))
@@ -523,7 +523,7 @@ bool TraceFmtDcdImpl::extractFrame()
             {
                 f_sync_bytes += 4;
                 dataPtr += 4;
-                cont_process = (bool)(dataPtr < eodPtr);
+                cont_process = (bool)(dataPtr + sizeof(uint32_t) < eodPtr);
             }
         }

@@ -566,7 +566,7 @@ bool TraceFmtDcdImpl::extractFrame()
             }

             dataPtr += 2;
-            cont_process = (bool)(dataPtr < eodPtr);
+            cont_process = (bool)(dataPtr + sizeof(uint32_t) < eodPtr);
         }

         // if we hit the end of data but still have a complete frame waiting,
--

[mikel-armbb]

There is a possibility of an issue when supplying 2 byte aligned data to this section of the decoder.
This area has been updated to fix the handling of FSYNCs and HSYNCs .

Released in 1.3.3