From 232282fd8f9c21eefe8d2d2b96cdbbb172fe7b7c Mon Sep 17 00:00:00 2001
From: Su Laus <sulau@freenet.de>
Date: Tue, 8 Mar 2022 17:02:44 +0000
Subject: [PATCH] tiffcrop: fix issue #380 and #382 heap buffer overflow in
 extractImageSection

Conflict:NA
Reference:https://gitlab.com/freedesktop-sdk/mirrors/gitlab/libtiff/libtiff/-/commit/232282fd8f9c21eefe8d2d2b96cdbbb172fe7b7c
---
 tools/tiffcrop.c | 92 +++++++++++++++++++-----------------------------
 1 file changed, 36 insertions(+), 56 deletions(-)

diff --git a/tools/tiffcrop.c b/tools/tiffcrop.c
index f2e5474a..e62bcc71 100644
--- a/tools/tiffcrop.c
+++ b/tools/tiffcrop.c
@@ -105,8 +105,8 @@
  *                of messages to monitor progress without enabling dump logs.
  */
 
-static   char tiffcrop_version_id[] = "2.4";
-static   char tiffcrop_rev_date[] = "12-13-2010";
+static   char tiffcrop_version_id[] = "2.4.1";
+static   char tiffcrop_rev_date[] = "03-03-2010";
 
 #include "tif_config.h"
 #include "libport.h"
@@ -6739,10 +6739,10 @@ extractImageSection(struct image_data *image, struct pageseg *section,
 #ifdef DEVELMODE
   uint32_t    img_length;
 #endif
-  uint32_t    j, shift1, shift2, trailing_bits;
+  uint32_t    j, shift1, trailing_bits;
   uint32_t    row, first_row, last_row, first_col, last_col;
   uint32_t    src_offset, dst_offset, row_offset, col_offset;
-  uint32_t    offset1, offset2, full_bytes;
+  uint32_t    offset1, full_bytes;
   uint32_t    sect_width;
 #ifdef DEVELMODE
   uint32_t    sect_length;
@@ -6752,7 +6752,6 @@ extractImageSection(struct image_data *image, struct pageseg *section,
 #ifdef DEVELMODE
   int      k;
   unsigned char bitset;
-  static char *bitarray = NULL;
 #endif
 
   img_width = image->width;
@@ -6770,17 +6769,12 @@ extractImageSection(struct image_data *image, struct pageseg *section,
   dst_offset = 0;
 
 #ifdef DEVELMODE
-  if (bitarray == NULL)
-    {
-    if ((bitarray = (char *)malloc(img_width)) == NULL)
-      {
-      TIFFError ("", "DEBUG: Unable to allocate debugging bitarray");
-      return (-1);
-      }
-    }
+  char bitarray[39];
 #endif
 
-  /* rows, columns, width, length are expressed in pixels */
+  /* rows, columns, width, length are expressed in pixels
+   * first_row, last_row, .. are index into image array starting at 0 to width-1,
+   * last_col shall be also extracted.  */
   first_row = section->y1;
   last_row  = section->y2;
   first_col = section->x1;
@@ -6790,9 +6784,14 @@ extractImageSection(struct image_data *image, struct pageseg *section,
 #ifdef DEVELMODE
   sect_length = last_row - first_row + 1;
 #endif
-  img_rowsize = ((img_width * bps + 7) / 8) * spp;
-  full_bytes = (sect_width * spp * bps) / 8;   /* number of COMPLETE bytes per row in section */
-  trailing_bits = (sect_width * bps) % 8;
+    /* The read function loadImage() used copy separate plane data into a buffer as interleaved
+     * samples rather than separate planes so the same logic works to extract regions
+     * regardless of the way the data are organized in the input file.
+     * Furthermore, bytes and bits are arranged in buffer according to COMPRESSION=1 and FILLORDER=1 
+     */
+    img_rowsize = (((img_width * spp * bps) + 7) / 8);    /* row size in full bytes of source image */
+    full_bytes = (sect_width * spp * bps) / 8;            /* number of COMPLETE bytes per row in section */
+    trailing_bits = (sect_width * spp * bps) % 8;         /* trailing bits within the last byte of destination buffer */
 
 #ifdef DEVELMODE
     TIFFError ("", "First row: %"PRIu32", last row: %"PRIu32", First col: %"PRIu32", last col: %"PRIu32"\n",
@@ -6805,10 +6804,9 @@ extractImageSection(struct image_data *image, struct pageseg *section,
 
   if ((bps % 8) == 0)
     {
-    col_offset = first_col * spp * bps / 8;
+    col_offset = (first_col * spp * bps) / 8;
     for (row = first_row; row <= last_row; row++)
       {
-      /* row_offset = row * img_width * spp * bps / 8; */
       row_offset = row * img_rowsize;
       src_offset = row_offset + col_offset;
 
@@ -6821,14 +6819,12 @@ extractImageSection(struct image_data *image, struct pageseg *section,
     }
   else
     { /* bps != 8 */
-    shift1  = spp * ((first_col * bps) % 8);
-    shift2  = spp * ((last_col * bps) % 8);
+    shift1 = ((first_col * spp * bps) % 8);           /* shift1 = bits to skip in the first byte of source buffer*/
     for (row = first_row; row <= last_row; row++)
       {
       /* pull out the first byte */
       row_offset = row * img_rowsize;
-      offset1 = row_offset + (first_col * bps / 8);
-      offset2 = row_offset + (last_col * bps / 8);
+      offset1 = row_offset + ((first_col * spp * bps) / 8);   /* offset1 = offset into source of byte with first bits to be extracted */
 
 #ifdef DEVELMODE
       for (j = 0, k = 7; j < 8; j++, k--)
@@ -6840,12 +6836,12 @@ extractImageSection(struct image_data *image, struct pageseg *section,
       sprintf(&bitarray[9], " ");
       for (j = 10, k = 7; j < 18; j++, k--)
         {
-        bitset = *(src_buff + offset2) & (((unsigned char)1 << k)) ? 1 : 0;
+        bitset = *(src_buff + offset1 + full_bytes) & (((unsigned char)1 << k)) ? 1 : 0;
         sprintf(&bitarray[j], (bitset) ? "1" : "0");
         }
       bitarray[18] = '\0';
-      TIFFError ("", "Row: %3d Offset1: %"PRIu32",  Shift1: %"PRIu32",    Offset2: %"PRIu32",  Shift2:  %"PRIu32"\n", 
-                 row, offset1, shift1, offset2, shift2); 
+      TIFFError ("", "Row: %3d Offset1: %"PRIu32",  Shift1: %"PRIu32",    Offset2: %"PRIu32",  Trailing_bits:  %"PRIu32"\n", 
+                 row, offset1, shift1, offset1+full_bytes, trailing_bits); 
 #endif
 
       bytebuff1 = bytebuff2 = 0;
@@ -6869,11 +6865,12 @@ extractImageSection(struct image_data *image, struct pageseg *section,
 
         if (trailing_bits != 0)
           {
-	  bytebuff2 = src_buff[offset2] & ((unsigned char)255 << (7 - shift2));
+      /* Only copy higher bits of samples and mask lower bits of not wanted column samples to zero */
+	  bytebuff2 = src_buff[offset1 + full_bytes] & ((unsigned char)255 << (8 - trailing_bits));
           sect_buff[dst_offset] = bytebuff2;
 #ifdef DEVELMODE
 	  TIFFError ("", "        Trailing bits src offset:  %8"PRIu32", Dst offset: %8"PRIu32"\n",
-                              offset2, dst_offset); 
+          offset1 + full_bytes, dst_offset);
           for (j = 30, k = 7; j < 38; j++, k--)
             {
             bitset = *(sect_buff + dst_offset) & (((unsigned char)1 << k)) ? 1 : 0;
@@ -6892,8 +6889,10 @@ extractImageSection(struct image_data *image, struct pageseg *section,
 #endif
         for (j = 0; j <= full_bytes; j++) 
           {
-	  bytebuff1 = src_buff[offset1 + j] & ((unsigned char)255 >> shift1);
-	  bytebuff2 = src_buff[offset1 + j + 1] & ((unsigned char)255 << (7 - shift1));
+          /* Skip the first shift1 bits and shift the source up by shift1 bits before save to destination.*/
+          /* Attention: src_buff size needs to be some bytes larger than image size, because could read behind image here. */
+          bytebuff1 = src_buff[offset1 + j] & ((unsigned char)255 >> shift1);
+          bytebuff2 = src_buff[offset1 + j + 1] & ((unsigned char)255 << (8 - shift1));
           sect_buff[dst_offset + j] = (bytebuff1 << shift1) | (bytebuff2 >> (8 - shift1));
           }
 #ifdef DEVELMODE
@@ -6909,36 +6908,17 @@ extractImageSection(struct image_data *image, struct pageseg *section,
 #endif
         dst_offset += full_bytes;
 
+        /* Copy the trailing_bits for the last byte in the destination buffer. 
+           Could come from one ore two bytes of the source buffer. */
         if (trailing_bits != 0)
           {
 #ifdef DEVELMODE
-	    TIFFError ("", "        Trailing bits   src offset: %8"PRIu32", Dst offset: %8"PRIu32"\n", offset1 + full_bytes, dst_offset);
-#endif
-	  if (shift2 > shift1)
-            {
-	    bytebuff1 = src_buff[offset1 + full_bytes] & ((unsigned char)255 << (7 - shift2));
-            bytebuff2 = bytebuff1 & ((unsigned char)255 << shift1);
-            sect_buff[dst_offset] = bytebuff2;
-#ifdef DEVELMODE
-	    TIFFError ("", "        Shift2 > Shift1\n"); 
+          TIFFError("", "        Trailing bits %4"PRIu32"   src offset: %8"PRIu32", Dst offset: %8"PRIu32"\n", trailing_bits, offset1 + full_bytes, dst_offset);
 #endif
+          /* More than necessary bits are already copied into last destination buffer, 
+           * only masking of last byte in destination buffer is necessary.*/ 
+          sect_buff[dst_offset] &= ((uint8_t)0xFF << (8 - trailing_bits));
             }
-          else
-            {
-	    if (shift2 < shift1)
-              {
-              bytebuff2 = ((unsigned char)255 << (shift1 - shift2 - 1));
-	      sect_buff[dst_offset] &= bytebuff2;
-#ifdef DEVELMODE
-	      TIFFError ("", "        Shift2 < Shift1\n"); 
-#endif
-              }
-#ifdef DEVELMODE
-            else
-	      TIFFError ("", "        Shift2 == Shift1\n"); 
-#endif
-            }
-	  }
 #ifdef DEVELMODE
 	  sprintf(&bitarray[28], " ");
 	  sprintf(&bitarray[29], " ");
@@ -7091,7 +7071,7 @@ writeImageSections(TIFF *in, TIFF *out, struct image_data *image,
     width  = sections[i].x2 - sections[i].x1 + 1;
     length = sections[i].y2 - sections[i].y1 + 1;
     sectsize = (uint32_t)
-	    ceil((width * image->bps + 7) / (double)8) * image->spp * length;
+	    ceil((width * image->bps * image->spp + 7) / (double)8) * length;
     /* allocate a buffer if we don't have one already */
     if (createImageSection(sectsize, sect_buff_ptr))
       {
-- 
GitLab