@@ -5,15 +5,53 @@ Note: This file refers to the source tarball. Things described here may differ

 ## 0.103.1

-ClamAV 0.103.1 is a bug patch release to address the following issues.

+ClamAV 0.103.1 is a patch release with the following fixes and improvements.

-- Inter-process file descriptor passing for clamonacc was non-functional in previous

-  versions due to a bug introduced by the switch to curl for communicating with clamd.

-  On Linux, passing file descriptors from one process to another is handled by the kernel,

-  so we reverted clamonacc to use standard system calls for socket communication when

-  fd passing is enabled.

+### Notable changes

+

+- Added a new scan option to alert on broken media (graphics) file formats.

+  This feature mitigates the risk of malformed media files intended to exploit

+  vulnerabilities in other software.

+  At present media validation exists for JPEG, TIFF, PNG, and GIF files.

+  To enable this feature, set `AlertBrokenMedia yes` in clamd.conf, or use

+  the `--alert-broken-media` option when using `clamscan`.

+  These options are disabled by default in this patch release, but may be

+  enabled in a subsequent release.

+  Application developers may enable this scan option by enabling

+  `CL_SCAN_HEURISTIC_BROKEN_MEDIA` for the `heuristic` scan option bit field.

+

+- Added CL_TYPE_TIFF, CL_TYPE_JPEG types to match GIF, PNG typing behavior.

+  BMP and JPEG 2000 files will continue to detect as CL_TYPE_GRAPHICS because

+  ClamAV does not yet have BMP or JPEG 2000 format checking capabilities.

+

+### Bug fixes

+

+- Fixed PNG parser logic bugs that caused an excess of parsing errors and fixed

+  a stack exhaustion issue affecting some systems when scanning PNG files.

+  PNG file type detection was disabled via signature database update for

+  ClamAV version 0.103.0 to mitigate the effects from these bugs.

+

+- Fixed an issue where PNG and GIF files no longer work with Target:5 graphics

+  signatures if detected as CL_TYPE_PNG/GIF rather than as CL_TYPE_GRAPHICS.

+  Target types now support up to 10 possible file types to make way for

+  additional graphics types in future releases.

+

+- Fixed clamonacc's `--fdpass` option.

-- Fix clamonacc stack corruption issue on some systems when using an older

+  File descriptor passing (or "fd-passing") is a mechanism by which clamonacc

+  and clamdscan may transfer an open file to clamd to scan, even if clamd is

+  running as a non-privileged user and wouldn't otherwise have read-access to

+  the file. This enables clamd to scan all files without having to run clamd as

+  root. If possible, clamd should never be run as root so as to mitigate the

+  risk in case clamd is somehow compromised while scanning malware.

+

+  Interprocess file descriptor passing for clamonacc was broken since version

+  0.102.0 due to a bug introduced by the switch to curl for communicating with

+  clamd. On Linux, passing file descriptors from one process to another is

+  handled by the kernel, so we reverted clamonacc to use standard system calls

+  for socket communication when fd passing is enabled.

+

+- Fixed a clamonacc stack corruption issue on some systems when using an older

   version of libcurl. Patch courtesy of Emilio Pozuelo Monfort.

 - Allow clamscan and clamdscan scans to proceed even if the realpath lookup

@@ -33,6 +71,22 @@ ClamAV 0.103.1 is a bug patch release to address the following issues.

 - Fixed an issue where freshclam database validation didn't work correctly when

   run in daemon mode on Linux/Unix.

+### Other improvements

+

+- Scanning JPEG, TIFF, PNG, and GIF files will no longer return "parse" errors

+  when file format validation fails. Instead, the scan will alert with the

+  "Heuristics.Broken.Media" signature prefix and a descriptive suffix to

+  indicate the issue, provided that the "alert broken media" feature is enabled.

+

+- GIF format validation will no longer fail if the GIF image is missing the

+  trailer byte, as this appears to be a relatively common issue in otherwise

+  functional GIF files.

+

+- Added a TIFF dynamic configuration (DCONF) option, which was missing.

+  This will allow us to disable TIFF format validation via signature database

+  update in the event that it proves to be problematic.

+  This feature already exists for many other file types.

+

 ### Acknowledgements

 The ClamAV team thanks the following individuals for their code submissions:

@@ -1203,6 +1203,11 @@ int recvloop(int *socketds, unsigned nsockets, struct cl_engine *engine, unsigne

         }

     }

+    if (optget(opts, "AlertBrokenMedia")->enabled) {

+        options.heuristic |= CL_SCAN_HEURISTIC_BROKEN_MEDIA;

+        logg("Media (Graphics) Format Validatation enabled\n");

+    }

+

     if (optget(opts, "ScanMail")->enabled) {

         logg("Mail files support enabled.\n");

         options.parse |= CL_SCAN_PARSE_MAIL;

@@ -78,7 +78,7 @@ int main(int argc, char **argv)

         exit(2);

 #if !defined(_WIN32)

-    if(!setlocale(LC_CTYPE, "")) {

+    if (!setlocale(LC_CTYPE, "")) {

         mprintf("^Failed to set locale\n");

     }

 #if !defined(C_BEOS)

@@ -299,6 +299,7 @@ void help(void)

     mprintf("    --scan-hwp3[=yes(*)/no]              Scan HWP3 files\n");

     mprintf("    --scan-archive[=yes(*)/no]           Scan archive files (supported by libclamav)\n");

     mprintf("    --alert-broken[=yes/no(*)]           Alert on broken executable files (PE & ELF)\n");

+    mprintf("    --alert-broken-media[=yes/no(*)]     Alert on broken graphics files (JPEG, TIFF, PNG, GIF)\n");

     mprintf("    --alert-encrypted[=yes/no(*)]        Alert on encrypted archives and documents\n");

     mprintf("    --alert-encrypted-archive[=yes/no(*)] Alert on encrypted archives\n");

     mprintf("    --alert-encrypted-doc[=yes/no(*)]    Alert on encrypted documents\n");

@@ -1070,6 +1070,10 @@ int scanmanager(const struct optstruct *opts)

         options.heuristic |= CL_SCAN_HEURISTIC_BROKEN;

     }

+    if (optget(opts, "alert-broken-media")->enabled) {

+        options.heuristic |= CL_SCAN_HEURISTIC_BROKEN_MEDIA;

+    }

+

     /* TODO: Remove deprecated option in a future feature release */

     if ((optget(opts, "block-encrypted")->enabled) ||

         (optget(opts, "alert-encrypted")->enabled)) {

@@ -477,6 +477,11 @@ Alert on broken executable files (PE & ELF).

 .br

 Default: no

 .TP

+\fBAlertBrokenMedia BOOL\fR

+Alert on broken graphics files (JPEG, TIFF, PNG, GIF).

+.br

+Default: no

+.TP

 \fBAlertEncrypted BOOL\fR

 Alert on encrypted archives and documents (encrypted .zip, .7zip, .rar, .pdf).

 .br

@@ -301,6 +301,11 @@ Example

 # Default: no

 #AlertBrokenExecutables yes

+# With this option clamav will try to detect broken media file (JPEG,

+# TIFF, PNG, GIF) and alert on them with a Broken.Media heuristic signature.

+# Default: no

+#AlertBrokenMedia yes

+

 # Alert on encrypted archives _and_ documents with heuristic signature

 # (encrypted .zip, .7zip, .rar, .pdf).

 # Default: no

@@ -191,8 +191,8 @@ struct cl_scan_options {

 #define CL_SCAN_HEURISTIC_STRUCTURED                0x200  /* data loss prevention options, i.e. alert when detecting personal information */

 #define CL_SCAN_HEURISTIC_STRUCTURED_SSN_NORMAL     0x400  /* alert when detecting social security numbers */

 #define CL_SCAN_HEURISTIC_STRUCTURED_SSN_STRIPPED   0x800  /* alert when detecting stripped social security numbers */

-

 #define CL_SCAN_HEURISTIC_STRUCTURED_CC             0x1000 /* alert when detecting credit card numbers */

+#define CL_SCAN_HEURISTIC_BROKEN_MEDIA              0x2000 /* alert if a file does not match the identified file format, works with JPEG, TIFF, GIF, PNG */

 /* mail scanning options */

 #define CL_SCAN_MAIL_PARTIAL_MESSAGE                0x1

@@ -134,6 +134,7 @@ static struct dconf_module modules[] = {

     {"OTHER", "LZW", OTHER_CONF_LZW, 1},

     {"OTHER", "GIF", OTHER_CONF_GIF, 1},

     {"OTHER", "PNG", OTHER_CONF_PNG, 1},

+    {"OTHER", "TIFF", OTHER_CONF_TIFF, 1},

     {"PHISHING", "ENGINE", PHISHING_CONF_ENGINE, 1},

     {"PHISHING", "ENTCONV", PHISHING_CONF_ENTCONV, 1},

@@ -169,27 +170,21 @@ struct cli_dconf *cli_dconf_init(void)

         if (!strcmp(modules[i].mname, "PE")) {

             if (modules[i].state)

                 dconf->pe |= modules[i].bflag;

-

         } else if (!strcmp(modules[i].mname, "ELF")) {

             if (modules[i].state)

                 dconf->elf |= modules[i].bflag;

-

         } else if (!strcmp(modules[i].mname, "MACHO")) {

             if (modules[i].state)

                 dconf->macho |= modules[i].bflag;

-

         } else if (!strcmp(modules[i].mname, "ARCHIVE")) {

             if (modules[i].state)

                 dconf->archive |= modules[i].bflag;

-

         } else if (!strcmp(modules[i].mname, "DOCUMENT")) {

             if (modules[i].state)

                 dconf->doc |= modules[i].bflag;

-

         } else if (!strcmp(modules[i].mname, "MAIL")) {

             if (modules[i].state)

                 dconf->mail |= modules[i].bflag;

-

         } else if (!strcmp(modules[i].mname, "OTHER")) {

             if (modules[i].state)

                 dconf->other |= modules[i].bflag;

@@ -127,6 +127,7 @@ struct cli_dconf {

 #define OTHER_CONF_LZW          0x400

 #define OTHER_CONF_PNG          0x800

 #define OTHER_CONF_GIF          0x1000

+#define OTHER_CONF_TIFF         0x2000

 /* Phishing flags */

 #define PHISHING_CONF_ENGINE  0x1

@@ -86,6 +86,8 @@ static const struct ftmap_s {

     { "CL_TYPE_GRAPHICS",     CL_TYPE_GRAPHICS     },

     { "CL_TYPE_GIF",          CL_TYPE_GIF          },

     { "CL_TYPE_PNG",          CL_TYPE_PNG          },

+    { "CL_TYPE_JPEG",         CL_TYPE_JPEG         },

+    { "CL_TYPE_TIFF",         CL_TYPE_TIFF         },

     { "CL_TYPE_RIFF",         CL_TYPE_RIFF         },

     { "CL_TYPE_BINHEX",       CL_TYPE_BINHEX       },

     { "CL_TYPE_TNEF",         CL_TYPE_TNEF         },

@@ -68,6 +68,8 @@ typedef enum cli_file {

     CL_TYPE_GRAPHICS,

     CL_TYPE_GIF,

     CL_TYPE_PNG,

+    CL_TYPE_JPEG,

+    CL_TYPE_TIFF,

     CL_TYPE_RIFF,

     CL_TYPE_BINHEX,

     CL_TYPE_TNEF,

@@ -36,7 +36,7 @@ print "  NULL\n};\n"

 */

 static const char *ftypes_int[] = {

-    "0:0:0000000c6a5020200d0a870a:JPEG2000:CL_TYPE_ANY:CL_TYPE_GRAPHICS",

+    "0:0:0000000c6a5020200d0a870a:JPEG2000:CL_TYPE_ANY:CL_TYPE_JPEG",

     "0:0:000001b3:MPEG video stream:CL_TYPE_ANY:CL_TYPE_IGNORED",

     "0:0:000001ba:MPEG sys stream:CL_TYPE_ANY:CL_TYPE_IGNORED",

     "0:0:1f8b:GZip:CL_TYPE_ANY:CL_TYPE_GZ",

@@ -90,7 +90,7 @@ static const char *ftypes_int[] = {

     "0:0:89504e47:PNG:CL_TYPE_ANY:CL_TYPE_PNG",

     "0:0:b6b9acaefeffffff:CryptFF:CL_TYPE_ANY:CL_TYPE_CRYPTFF",

     "0:0:d0cf11e0a1b11ae1:OLE2 container:CL_TYPE_ANY:CL_TYPE_MSOLE2",

-    "0:0:ffd8ff:JPEG:CL_TYPE_ANY:CL_TYPE_GRAPHICS",

+    "0:0:ffd8ff:JPEG:CL_TYPE_ANY:CL_TYPE_JPEG",

     "0:0:fffb90:MP3:CL_TYPE_ANY:CL_TYPE_IGNORED",

     "1:*:3c4120*(68|48)(72|52)4546:HTML data:CL_TYPE_ANY:CL_TYPE_HTML",

     "1:*:3c4120*(68|48)(72|52)6566:HTML data:CL_TYPE_ANY:CL_TYPE_HTML",

@@ -189,8 +189,8 @@ static const char *ftypes_int[] = {

     "1:0:3c3f786d6c2076657273696f6e3d22312e3022*70726f6769643d22576f72642e446f63756d656e74223f3e:Microsoft Word 2003 XML Document:CL_TYPE_ANY:CL_TYPE_XML_WORD:80",

     "1:0:3c3f786d6c2076657273696f6e3d22312e3022*3c576f726b626f6f6b:Microsoft Excel 2003 XML Document:CL_TYPE_ANY:CL_TYPE_XML_XL:80",

     "1:0:3c3f786d6c2076657273696f6e3d22312e3022*3c??3a576f726b626f6f6b:Microsoft Excel 2003 XML Document:CL_TYPE_ANY:CL_TYPE_XML_XL:80",

-    "0:0:49492a00:TIFF Little Endian:CL_TYPE_ANY:CL_TYPE_GRAPHICS:81",

-    "0:0:4d4d:TIFF Big Endian:CL_TYPE_ANY:CL_TYPE_GRAPHICS:81",

+    "0:0:49492a00:TIFF Little Endian:CL_TYPE_ANY:CL_TYPE_TIFF:81",

+    "0:0:4d4d:TIFF Big Endian:CL_TYPE_ANY:CL_TYPE_TIFF:81",

     "0:4:d0cf11e0a1b11ae1:HWP embedded OLE2:CL_TYPE_ANY:CL_TYPE_HWPOLE2",

     "0:0:48575020446f63756d656e742046696c652056332e3030201a0102030405:HWP 3.x Document:CL_TYPE_ANY:CL_TYPE_HWP3:82",

     "1:0:efbbbf3c3f786d6c2076657273696f6e3d22312e3022*3c4857504d4c:HWPML Document:CL_TYPE_ANY:CL_TYPE_XML_HWP:82",

@@ -187,7 +187,7 @@ static inline void fmap_unneed_ptr(fmap_t *m, const void *ptr, size_t len)

 }

 /**

- * @brief Read bytes from fmap at offset into destinatio buffer.

+ * @brief Read bytes from fmap at offset into destination buffer.

  *

  * @param m         fmap

  * @param dst       destination buffer

@@ -1,32 +1,75 @@

 /*

-*  Copyright (C) 2013-2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.

-*  Copyright (C) 2011-2013 Sourcefire, Inc.

-*

-*  Authors: Tomasz Kojm <tkojm@clamav.net>

-*

-*  This program is free software; you can redistribute it and/or modify

-*  it under the terms of the GNU General Public License version 2 as

-*  published by the Free Software Foundation.

-*

-*  This program is distributed in the hope that it will be useful,

-*  but WITHOUT ANY WARRANTY; without even the implied warranty of

-*  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

-*  GNU General Public License for more details.

-*

-*  You should have received a copy of the GNU General Public License

-*  along with this program; if not, write to the Free Software

-*  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,

-*  MA 02110-1301, USA.

-*/

+ *  Copyright (C) 2013-2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.

+ *  Copyright (C) 2011-2013 Sourcefire, Inc.

+ *

+ *  Authors: Tomasz Kojm <tkojm@clamav.net>, Aldo Mazzeo, Micah Snyder

+ *

+ *  This program is free software; you can redistribute it and/or modify

+ *  it under the terms of the GNU General Public License version 2 as

+ *  published by the Free Software Foundation.

+ *

+ *  This program is distributed in the hope that it will be useful,

+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of

+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

+ *  GNU General Public License for more details.

+ *

+ *  You should have received a copy of the GNU General Public License

+ *  along with this program; if not, write to the Free Software

+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,

+ *  MA 02110-1301, USA.

+ */

+

+/*

+ *  GIF Format

+ *  ----------

+ *

+ *  1. Signature: 3 bytes  ("GIF")

+ *

+ *  2. Version: 3 bytes  ("87a" or "89a")

+ *

+ *  3. Logical Screen Descriptor: 7 bytes (see `struct gif_screen_descriptor`)

+ *     (Opt.) Global Color Table: n bytes (defined in the Logical Screen Descriptor flags)

+ *

+ *  4. All subsequent blocks are precededed by the following 1-byte labels...

+ *

+ *     0x21:  Extension Introducer

+ *        0x01:  Opt. (0+) Plain Text Extension

+ *        0xF9:  Opt. (0+) Graphic Control Extension

+ *        0xFE:  Opt. (0+) Comment Extension

+ *        0xFF:  Opt. (0+) Application Extension

+ *

+ *        Note: Each extension has a size field followed by some data. After the

+ *              data may be a series of sub-blocks, each with a block size.

+ *              If there are no more sub-blocks, the size will be 0x00, meaning

+ *              there's no more blocks.

+ *              The Graphic Control Extension never has any sub-blocks.

+ *

+ *     0x2C:  Image Descriptor  (1 per image, unlimited images)

+ *        (Opt.) Local Color Table: n bytes (defined in the Image Descriptor flags)

+ *        (Req.) Table-based Image Data Block*

+ *

+ *        Note: Each image a series of data blocks of size 0-255 bytes each where

+ *              the first byte is the size of the data-block.

+ *              If there are no more data-blocks, the size will be 0x00, meaning

+ *              there's no more data.

+ *

+ *     0x3B:  Trailer (1 located at end of data stream)

+ *

+ *  Reference https://www.w3.org/Graphics/GIF/spec-gif89a.txt for the GIF spec.

+ */

 #if HAVE_CONFIG_H

 #include "clamav-config.h"

 #endif

+#include <math.h>

+#include <stdbool.h>

+

 #include "gif.h"

 #include "scanners.h"

 #include "clamav.h"

+/* clang-format off */

 #ifndef HAVE_ATTRIB_PACKED

 #define __attribute__(x)

 #endif

@@ -37,23 +80,53 @@

 #pragma pack 1

 #endif

-struct gif_screen_desc {

+/**

+ * @brief Logical Screen Descriptor

+ *

+ * This block immediately follows the  "GIF89a" magic bytes

+

+ * Flags contains packed fields which are as follows:

+ *  Global Color Table Flag     - 1 Bit

+ *  Color Resolution            - 3 Bits

+ *  Sort Flag                   - 1 Bit

+ *  Size of Global Color Table  - 3 Bits

+ */

+struct gif_screen_descriptor {

     uint16_t width;

     uint16_t height;

     uint8_t flags;

-    uint8_t bgcolor;

-    uint8_t aspect;

+    uint8_t bg_color_idx;

+    uint8_t pixel_aspect_ratio;

 } __attribute__((packed));

-struct gif_graphic_control_ext {

-    uint8_t blksize;

+#define GIF_SCREEN_DESC_FLAGS_MASK_HAVE_GLOBAL_COLOR_TABLE    0x80 /* If set, a Global Color Table will follow the Logical Screen Descriptor */

+#define GIF_SCREEN_DESC_FLAGS_MASK_COLOR_RESOLUTION           0x70 /* Number of bits per primary color available to the original image, minus 1. */

+#define GIF_SCREEN_DESC_FLAGS_MASK_SORT_FLAG                  0x08 /* Indicates whether the Global Color Table is sorted. */

+#define GIF_SCREEN_DESC_FLAGS_MASK_SIZE_OF_GLOBAL_COLOR_TABLE 0x07 /* If exists, the size = 3 * pow(2, this_field + 1), or: 3 * (1 << (this_field + 1)) */

+

+/**

+ * @brief Graphic Control Extension

+ *

+ */

+struct gif_graphic_control_extension {

+    uint8_t block_size;

     uint8_t flags;

     uint16_t delaytime;

-    uint8_t tcoloridx;

-    uint8_t blkterm;

+    uint8_t transparent_color_idx;

+    uint8_t block_terminator;

 } __attribute__((packed));

-struct gif_image_desc {

+/**

+ * @brief Image Descriptor

+ *

+ * Flags contains packed fields which are as follows:

+ *  Local Color Table Flag      - 1 Bit

+ *  Interlace Flag              - 1 Bit

+ *  Sort Flag                   - 1 Bit

+ *  Reserved                    - 2 Bits

+ *  Size of Local Color Table   - 3 Bits

+ */

+struct gif_image_descriptor {

     uint16_t leftpos;

     uint16_t toppos;

     uint16_t width;

@@ -61,99 +134,292 @@ struct gif_image_desc {

     uint8_t flags;

 } __attribute__((packed));

+#define GIF_IMAGE_DESC_FLAGS_MASK_HAVE_LOCAL_COLOR_TABLE    0x80 /* If set, a Global Color Table will follow the Logical Screen Descriptor */

+#define GIF_IMAGE_DESC_FLAGS_MASK_IS_INTERLACED             0x40 /* Indicates if the image is interlaced */

+#define GIF_IMAGE_DESC_FLAGS_MASK_SORT_FLAG                 0x20 /* Indicates whether the Local Color Table is sorted. */

+#define GIF_IMAGE_DESC_FLAGS_MASK_SIZE_OF_LOCAL_COLOR_TABLE 0x07 /* If exists, the size = 3 * pow(2, this_field + 1), or: 3 * (1 << (this_field + 1)) */

+

+/* Main labels */

+#define GIF_LABEL_EXTENSION_INTRODUCER                  0x21

+#define GIF_LABEL_GRAPHIC_IMAGE_DESCRIPTOR              0x2C

+#define GIF_LABEL_SPECIAL_TRAILER                       0x3B

+

+/* Extension labels (found after the Extension Introducer) */

+#define GIF_LABEL_GRAPHIC_PLAIN_TEXT_EXTENSION          0x01

+#define GIF_LABEL_CONTROL_GRAPHIC_CONTROL_EXTENSION     0xF9

+#define GIF_LABEL_SPECIAL_COMMENT_EXTENSION             0xFE

+#define GIF_LABEL_SPECIAL_APP_EXTENSION                 0xFF

+

+#define GIF_BLOCK_TERMINATOR 0x00 /* Used to indicate end of image data and also for end of extension sub-blocks */

+

 #ifdef HAVE_PRAGMA_PACK

 #pragma pack()

 #endif

 #ifdef HAVE_PRAGMA_PACK_HPPA

 #pragma pack

 #endif

-

-#define EC16(x) le16_to_host(x)

-

-#define GETDATA(v)                                                      \

-    {                                                                   \

-        if (fmap_readn(map, &v, offset, sizeof(v)) == sizeof(v)) {      \

-            offset += sizeof(v);                                        \

-        } else {                                                        \

-            cli_errmsg("cli_parsegif: Can't read file (truncated?)\n"); \

-            return CL_EPARSE;                                           \

-        }                                                               \

-    }

+/* clang-format on */

 cl_error_t cli_parsegif(cli_ctx *ctx)

 {

-    fmap_t *map         = *ctx->fmap;

-    unsigned char v     = 0;

-    unsigned int offset = 6;

-    struct gif_screen_desc screen_desc;

-    struct gif_image_desc image_desc;

-    cl_error_t retVal = CL_SUCCESS;

+    cl_error_t status = CL_SUCCESS;

+

+    fmap_t *map   = NULL;

+    size_t offset = 0;

+

+    const char *signature = NULL;

+    char version[4];

+    struct gif_screen_descriptor screen_desc;

+    size_t global_color_table_size = 0;

+    bool have_image_data           = false;

     cli_dbgmsg("in cli_parsegif()\n");

-    GETDATA(screen_desc);

-    cli_dbgmsg("GIF: Screen size %ux%u, gctsize: %u\n", EC16(screen_desc.width), EC16(screen_desc.height), screen_desc.flags & 0x7);

-    if (screen_desc.flags & 0x80)

-        offset += 3 * (1 << ((screen_desc.flags & 0x7) + 1));

+    if (NULL == ctx) {

+        cli_dbgmsg("GIF: passed context was NULL\n");

+        status = CL_EARG;

+        goto done;

+    }

+    map = *ctx->fmap;

+

+    /*

+     * Skip the "GIF" Signature and "87a" or "89a" Version.

+     */

+    if (NULL == (signature = fmap_need_off(map, offset, strlen("GIF")))) {

+        cli_dbgmsg("GIF: Can't read GIF magic bytes, not a GIF\n");

+        goto done;

+    }

+    offset += strlen("GIF");

+

+    if (0 != strncmp("GIF", signature, 3)) {

+        cli_dbgmsg("GIF: First 3 bytes not 'GIF', not a GIF\n");

+        goto done;

+    }

+

+    if (3 != fmap_readn(map, &version, offset, strlen("89a"))) {

+        cli_dbgmsg("GIF: Can't read GIF format version, not a GIF\n");

+        goto done;

+    }

+    offset += strlen("89a");

+

+    version[3] = '\0';

+    cli_dbgmsg("GIF: Version: %s\n", version);

+

+    /*

+     * Read the Logical Screen Descriptor

+     */

+    if (fmap_readn(map, &screen_desc, offset, sizeof(screen_desc)) != sizeof(screen_desc)) {

+        cli_errmsg("GIF: Can't read logical screen description, file truncated?\n");

+        cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.TruncatedScreenDescriptor");

+        status = CL_EPARSE;

+        goto scan_overlay;

+    }

+    offset += sizeof(screen_desc);

+

+    cli_dbgmsg("GIF: Screen Size: %u width x %u height.\n",

+               le16_to_host(screen_desc.width),

+               le16_to_host(screen_desc.height));

+

+    if (screen_desc.flags & GIF_SCREEN_DESC_FLAGS_MASK_HAVE_GLOBAL_COLOR_TABLE) {

+        global_color_table_size = 3 * (1 << ((screen_desc.flags & GIF_SCREEN_DESC_FLAGS_MASK_SIZE_OF_GLOBAL_COLOR_TABLE) + 1));

+        cli_dbgmsg("GIF: Global Color Table size: %u\n", global_color_table_size);

+

+        if (offset + (size_t)global_color_table_size > map->len) {

+            cli_errmsg("GIF: EOF in the middle of the global color table, file truncated?\n");

+            cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.TruncatedGlobalColorTable");

+            status = CL_EPARSE;

+            goto scan_overlay;

+        }

+        offset += global_color_table_size;

+    } else {

+        cli_dbgmsg("GIF: No Global Color Table.\n");

+    }

     while (1) {

-        GETDATA(v);

-        if (v == 0x21) {

-            GETDATA(v);

-            if (v == 0xf9) {

-                offset += sizeof(struct gif_graphic_control_ext);

+        uint8_t block_label = 0;

+

+        /*

+         * Get the block label

+         */

+        if (fmap_readn(map, &block_label, offset, sizeof(block_label)) != sizeof(block_label)) {

+            if (have_image_data) {

+                /* Users have identified that GIF's lacking the image trailer are surprisingly common,

+                   can be rendered, and should be allowed. */

+                cli_dbgmsg("GIF: Missing GIF trailer, slightly (but acceptably) malformed.\n");

             } else {

-                while (1) {

-                    GETDATA(v);

-                    if (!v)

-                        break;

+                cli_errmsg("GIF: Can't read block label, EOF before image data. File truncated?\n");

+                cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.MissingImageData");

+            }

+            status = CL_EPARSE;

+            goto scan_overlay;

+        }

+        offset += sizeof(block_label);

-                    if (offset + v > map->len) {

-                        retVal = CL_EPARSE;

-                        goto scan_overlay;

+        if (block_label == GIF_LABEL_SPECIAL_TRAILER) {

+            /*

+             * Trailer (end of data stream)

+             */

+            cli_dbgmsg("GIF: Trailer (End of stream)\n");

+            goto scan_overlay;

+        }

+

+        switch (block_label) {

+            case GIF_LABEL_EXTENSION_INTRODUCER: {

+                uint8_t extension_label = 0;

+                cli_dbgmsg("GIF: Extension introducer:\n");

+

+                if (fmap_readn(map, &extension_label, offset, sizeof(extension_label)) != sizeof(extension_label)) {

+                    cli_errmsg("GIF: Failed to read the extension block label, file truncated?\n");

+                    cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.TruncatedExtension");

+                    status = CL_EPARSE;

+                    goto scan_overlay;

+                }

+                offset += sizeof(extension_label);

+

+                if (extension_label == GIF_LABEL_CONTROL_GRAPHIC_CONTROL_EXTENSION) {

+                    cli_dbgmsg("GIF:   Graphic control extension!\n");

+

+                    /* The size of a graphic control extension block is fixed, we can skip it quickly */

+                    offset += sizeof(struct gif_graphic_control_extension);

+                } else {

+                    switch (extension_label) {

+                        case GIF_LABEL_GRAPHIC_PLAIN_TEXT_EXTENSION:

+                            cli_dbgmsg("GIF:   Plain text extension\n");

+                            break;

+                        case GIF_LABEL_SPECIAL_COMMENT_EXTENSION:

+                            cli_dbgmsg("GIF:   Special comment extension\n");

+                            break;

+                        case GIF_LABEL_SPECIAL_APP_EXTENSION:

+                            cli_dbgmsg("GIF:   Special app extension\n");

+                            break;

+                        default:

+                            cli_dbgmsg("GIF:   Unfamiliar extension, label: 0x%x\n", extension_label);

+                    }

+

+                    while (1) {

+                        /*

+                         * Skip over the extension and any sub-blocks,

+                         * Try to read the block size for each sub-block to skip them.

+                         */

+                        uint8_t extension_block_size = 0;

+                        if (fmap_readn(map, &extension_block_size, offset, sizeof(extension_block_size)) != sizeof(extension_block_size)) {

+                            cli_errmsg("GIF: EOF while attempting to read the block size for an extension, file truncated?\n");

+                            cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.TruncatedExtension");

+                            status = CL_EPARSE;

+                            goto scan_overlay;

+                        } else {

+                            offset += sizeof(extension_block_size);

+                        }

+                        if (extension_block_size == GIF_BLOCK_TERMINATOR) {

+                            cli_dbgmsg("GIF:     No more sub-blocks for this extension.\n");

+                            break;

+                        } else {

+                            cli_dbgmsg("GIF:     Found sub-block of size %d\n", extension_block_size);

+                        }

+

+                        if (offset + (size_t)extension_block_size > map->len) {

+                            cli_errmsg("GIF: EOF in the middle of a graphic control extension sub-block, file truncated?\n");

+                            cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.TruncatedExtensionSubBlock");

+                            status = CL_EPARSE;

+                            goto scan_overlay;

+                        }

+                        offset += extension_block_size;

                     }

-                    offset += v;

                 }

+                break;

             }

-        } else if (v == 0x2c) {

-            GETDATA(image_desc);

-            cli_dbgmsg("GIF: Image size %ux%u, left pos: %u, top pos: %u\n", EC16(image_desc.width), EC16(image_desc.height), EC16(image_desc.leftpos), EC16(image_desc.toppos));

+            case GIF_LABEL_GRAPHIC_IMAGE_DESCRIPTOR: {

+                struct gif_image_descriptor image_desc;

+                size_t local_color_table_size = 0;

-            offset++;

-            if (image_desc.flags & 0x80)

-                offset += 3 * (1 << ((image_desc.flags & 0x7) + 1));

+                cli_dbgmsg("GIF: Found an image descriptor.\n");

+                if (fmap_readn(map, &image_desc, offset, sizeof(image_desc)) != sizeof(image_desc)) {

+                    cli_errmsg("GIF: Can't read image descriptor, file truncated?\n");

+                    cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.TruncatedImageDescriptor");

+                    status = CL_EPARSE;

+                    goto scan_overlay;

+                } else {

+                    offset += sizeof(image_desc);

+                }

+                cli_dbgmsg("GIF:   Image size: %u width x %u height, left pos: %u, top pos: %u\n",

+                           le16_to_host(image_desc.width),

+                           le16_to_host(image_desc.height),

+                           le16_to_host(image_desc.leftpos),

+                           le16_to_host(image_desc.toppos));

-            while (1) {

-                GETDATA(v);

-                if (!v)

-                    break;

+                if (image_desc.flags & GIF_IMAGE_DESC_FLAGS_MASK_HAVE_LOCAL_COLOR_TABLE) {

+                    local_color_table_size = 3 * (1 << ((image_desc.flags & GIF_IMAGE_DESC_FLAGS_MASK_SIZE_OF_LOCAL_COLOR_TABLE) + 1));

+                    cli_dbgmsg("GIF:     Found a Local Color Table (size: %u)\n", local_color_table_size);

+                    offset += local_color_table_size;

+                } else {

+                    cli_dbgmsg("GIF:     No Local Color Table.\n");

+                }

-                if (offset + v > map->len) {

-                    retVal = CL_EPARSE;

-                    goto scan_overlay;

+                /*

+                 * Parse the image data.

+                 */

+                offset++; /* Skip over the LZW Minimum Code Size uint8_t */

+

+                while (1) {

+                    /*

+                     * Skip over the image data block(s).

+                     * Try to read the block size for each image data sub-block to skip them.

+                     */

+                    uint8_t image_data_block_size = 0;

+                    if (fmap_readn(map, &image_data_block_size, offset, sizeof(image_data_block_size)) != sizeof(image_data_block_size)) {

+                        cli_errmsg("GIF: EOF while attempting to read the block size for an image data block, file truncated?\n");

+                        cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.TruncatedImageDataBlock");

+                        status = CL_EPARSE;

+                        goto scan_overlay;

+                    } else {

+                        offset += sizeof(image_data_block_size);

+                    }

+                    if (image_data_block_size == GIF_BLOCK_TERMINATOR) {

+                        cli_dbgmsg("GIF:     No more data sub-blocks for this image.\n");

+                        break;

+                    } else {

+                        cli_dbgmsg("GIF:     Found a sub-block of size %d\n", image_data_block_size);

+                    }

+

+                    if (offset + (size_t)image_data_block_size > map->len) {

+                        cli_errmsg("GIF: EOF in the middle of an image data sub-block, file truncated?\n");

+                        cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.TruncatedImageDataBlock");

+                        status = CL_EPARSE;

+                        goto scan_overlay;

+                    }

+                    offset += image_data_block_size;

                 }

-                offset += v;

+                have_image_data = true;

+                break;

+            }

+            default: {

+                // An unknown code: break.

+                cli_errmsg("GIF: Found an unfamiliar block label: 0x%x\n", block_label);

+                cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.GIF.UnknownBlockLabel");

+                status = CL_EPARSE;

+                goto scan_overlay;

             }

-        } else if (v == 0x3b) {

-            break;

-        } else {

-            // An unknown code: break.

-            retVal = CL_EPARSE;

-            goto scan_overlay;

         }

     }

 scan_overlay:

-    // Some recovery (I saw some "GIF89a;" or things like this)

-    if (retVal == CL_EPARSE &&

-        offset == (6 + sizeof(screen_desc) + 1))

-        offset = 6;

+    if (status == CL_EPARSE) {

+        /* We added with cli_append_possibly_unwanted so it will alert at the end if nothing else matches. */

+        status = CL_CLEAN;

+

+        // Some recovery (I saw some "GIF89a;" or things like this)

+        if (offset == (strlen("GIF89a") + sizeof(screen_desc) + 1)) {

+            offset = strlen("GIF89a");

+        }

+    }

     // Is there an overlay?

     if (offset < map->len) {

-        cl_error_t recRetVal = cli_magic_scan_nested_fmap_type(map, offset, map->len - offset, ctx, CL_TYPE_ANY, NULL);

-        retVal               = recRetVal != CL_SUCCESS ? recRetVal : retVal;

+        cli_dbgmsg("GIF: Found extra data after the end of the GIF data stream: %zu bytes, we'll scan it!\n", map->len - offset);

+        cl_error_t nested_scan_result = cli_magic_scan_nested_fmap_type(map, offset, map->len - offset, ctx, CL_TYPE_ANY, NULL);

+        status                        = nested_scan_result != CL_SUCCESS ? nested_scan_result : status;

     }

-    return retVal;

+done:

+    return status;

 }

@@ -37,148 +37,196 @@

 #include "jpeg.h"

 #include "clamav.h"

-#define GETBYTE(v)                                                   \

-    if (fmap_readn(map, &v, offset, sizeof(v)) == sizeof(v)) {       \

-        offset += sizeof(v);                                         \

-    } else {                                                         \

-        cli_errmsg("cli_parsejpeg: Can't read file (corrupted?)\n"); \

-        return CL_EPARSE;                                            \

-    }

-

 cl_error_t cli_parsejpeg(cli_ctx *ctx)

 {

-    fmap_t *map = *ctx->fmap;

-    unsigned char marker, prev_marker, prev_segment = 0, v1, v2, buff[8];

+    cl_error_t status = CL_CLEAN;

+

+    fmap_t *map = NULL;

+    unsigned char marker, prev_marker, prev_segment = 0, buff[8];

+    uint16_t len_u16;

     unsigned int offset = 0, i, len, comment = 0, segment = 0, app = 0;

     cli_dbgmsg("in cli_parsejpeg()\n");

+    if (NULL == ctx) {

+        cli_dbgmsg("JPEG: passed context was NULL\n");

+        status = CL_EARG;

+        goto done;

+    }

+    map = *ctx->fmap;

+

     if (fmap_readn(map, buff, offset, 4) != 4)

-        return CL_SUCCESS; /* Ignore */

+        goto done; /* Ignore */

     if (!memcmp(buff, "\xff\xd8\xff", 3))

         offset = 2;

     else if (!memcmp(buff, "\xff\xd9\xff\xd8", 4))

         offset = 4;

     else

-        return CL_SUCCESS; /* Not a JPEG file */

+        goto done; /* Not a JPEG file */

     while (1) {

         segment++;

         prev_marker = 0;

         for (i = 0; offset < map->len && i < 16; i++) {

-            GETBYTE(marker);

+            if (fmap_readn(map, &marker, offset, sizeof(marker)) == sizeof(marker)) {

+                offset += sizeof(marker);

+            } else {

+                cli_errmsg("JPEG: Failed to read marker, file corrupted?\n");

+                cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.JPEG.CantReadMarker");

+                goto done;

+            }

+

             if (prev_marker == 0xff && marker != 0xff)

                 break;

             prev_marker = marker;

         }

         if (i == 16) {

-            cli_warnmsg("cli_parsejpeg: Spurious bytes before segment %u\n", segment);

-            return CL_EPARSE;

+            cli_warnmsg("JPEG: Spurious bytes before segment %u\n", segment);

+            cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.JPEG.SpuriousBytesBeforeSegment");

+            status = CL_EPARSE;

+            goto done;

         }

-        if (offset == map->len) {

-            cli_warnmsg("cli_parsejpeg: Error looking for marker\n");

-            return CL_EPARSE;

+

+        if (fmap_readn(map, &len_u16, offset, sizeof(len_u16)) != sizeof(len_u16)) {

+            cli_errmsg("JPEG: Failed to read the segment size, file corrupted?\n");

+            cli_append_possibly_unwanted(ctx, "Heuristics.Broken.Media.JPEG.CantReadSegmentSize");

+            goto done;

         }

-        GETBYTE(v1);

-        GETBYTE(v2);

-        len = (unsigned int)(v1 << 8) | v2;

+        len = (unsigned int)be16_to_host(len_u16);

         cli_dbgmsg("JPEG: Marker %02x, length %u\n", marker, len);

+

         if (len < 2) {

-            cli_warnmsg("cli_parsejpeg: Invalid segment size\n");

-            return CL_EPARSE;