simplify BOT construction

avoid overflow for length value in dcm_parse_encapsulated_frame

Author: weanti

src/dicom-parse.c

@@ -955,65 +955,38 @@ bool dcm_parse_pixeldata_offsets(DcmError **error,
         // each frame
 
         dcm_log_info("building Offset Table from Pixel Data");
+	// by definition the first offset is 0
+	offsets[0] = 0;
 
         // 0 in the BOT is the offset to the start of frame 1, ie. here
         *first_frame_offset = position;
-        // each frame may consist of several fragments, so we need to scan each fragment to find the next frame
-        if (!read_tag(&state, &tag, &position)) {
-            dcm_error_set(error, DCM_ERROR_CODE_PARSE,
-                          "building BasicOffsetTable failed",
-                          "failed to read tag");
-            return false;
-        }
-        // all fragments belong to the only frame
-        if ( num_frames == 1 ) {
-            // according to the standard the first frame's offset is 0
-            offsets[0] = 0;
-        } else {
-            // 1 fragment shall contain 1 frame
-            int fragment_idx = 0;
-            offsets[fragment_idx] = 0; // by definition the first fragment is at offset 0
-            fragment_idx++;
-            while( tag != TAG_SQ_DELIM ) {
-                if ( tag != TAG_ITEM || !read_uint32(&state, &length, &position)) {
-                    dcm_error_set(error, DCM_ERROR_CODE_PARSE,
-                                  "building BasicOffsetTable failed",
-                                  "failed to read fragment");
-                    return false;
-                }
-                // skip actual content to find next offset
-                dcm_seekcur(&state, length, &position);
-                if (!read_tag(&state, &tag, &position)) {
-                    dcm_error_set(error, DCM_ERROR_CODE_PARSE,
-                                  "building BasicOffsetTable failed",
-                                  "failed to read tag");
-                    return false;
-                }
-                if ( fragment_idx < num_frames ) {
-                    offsets[fragment_idx] = offsets[fragment_idx-1] + 8/*tag and length field size*/ + length;
-                }
-                fragment_idx++;
-            }
-            // fragment_idx shall equal to num_frames+1 at the end
-            fragment_idx--;
-            if ( fragment_idx > num_frames ) {
-                dcm_error_set(error, DCM_ERROR_CODE_INVALID,
-                              "building BasicOffsetTable failed",
-                              "Too many fragments" );
+	for (int i = 1; i < num_frames; i++) {
+            if (!read_tag(&state, &tag, &position) ||
+                !read_uint32(&state, &length, &position)) {
                 return false;
             }
 
-            if ( num_frames < fragment_idx ) {
-                dcm_error_set(error, DCM_ERROR_CODE_INVALID,
-                              "building BasicOffsetTable failed",
-                              "Too many frames" );
+            if (tag == TAG_SQ_DELIM) {
+                dcm_error_set(error, DCM_ERROR_CODE_PARSE,
+                              "reading BasicOffsetTable failed",
+                              "too few frames in PixelData");
                 return false;
             }
-
-            if ( !read_uint32(&state, &length, &position ) || length != 0 ) {
+         
+            if (tag != TAG_ITEM) {
                 dcm_error_set(error, DCM_ERROR_CODE_PARSE,
                               "building BasicOffsetTable failed",
-                              "Sequence Delimiter Tag failure" );
+                              "frame Item #%d has wrong tag '%08x'",
+                              i + 1,
+                              tag);
+                return false;
+            }
+            
+            // step back to the start of the item for this frame
+            offsets[i] = position - 8;
+            
+            // and seek forward over the value
+            if (!dcm_seekcur(&state, length, &position)) {
                 return false;
             }
         }
@@ -1070,6 +1043,7 @@ char *dcm_parse_encapsulated_frame(DcmError **error,
     *length = 0;
     uint32_t tag;
     uint32_t fragment_length = 0;
+    uint64_t frame_length = 0;
 
     // first determine the total length of bytes to be read
     while(position < frame_end_offset) {
@@ -1089,10 +1063,13 @@ char *dcm_parse_encapsulated_frame(DcmError **error,
             return NULL;
         }
         dcm_seekcur(&state, fragment_length, &position);
-        *length += fragment_length;
+        frame_length += fragment_length;
+    }
+    if ( frame_length > 0xFFFFFFFF ) {
+        return NULL;
     }
 
-    char *value = DCM_MALLOC(error, *length);
+    char *value = DCM_MALLOC(error, frame_length);
     if (value == NULL) {
         return NULL;
     }
@@ -1115,6 +1092,6 @@ char *dcm_parse_encapsulated_frame(DcmError **error,
         }
         fragment += fragment_length;
     }
-
+    *length = (uint32_t) frame_length;
     return value;
 }