[Clamav-devel] Big endian issues

Reinhard Max max at suse.de
Mon Mar 13 17:47:57 UTC 2023


Hi,

this is a followup to the "Testing for Big Endian Architectures" 
thread on the clamav-users list in January.

I ran into the same test failures on big endian architectures when 
upgrading ClamAV to 1.0.1 for openSUSE. In addition to the fixes for 
the peinfo->dirs[1] array from the Debian patch I found some more 
members of peinfo and sub-structures that appeared to be missing 
endianness correction in pe.c. But they apparently didn't break any 
existing test. Some of them were already tagged with comments telling 
that endianness corrections are still missing here.

Find below an enhanced patch that also covers these cases, but I am 
not sure if I found them all.

Additionally I wonder if it wouldn't be better to put all these 
conversions into one place and call them after populating peinfo 
instead of scattering them across the whole file whenever one of the 
affected field gets read.

cu
Reinhard

--- libclamav/pe.c.orig
+++ libclamav/pe.c
@@ -2422,22 +2422,20 @@ static cl_error_t hash_imptbl(cli_ctx *c

      /* If the PE doesn't have an import table then skip it. This is an
       * uncommon case but can happen. */
-    if (peinfo->dirs[1].VirtualAddress == 0 || peinfo->dirs[1].Size == 0) {
+    if (EC32(peinfo->dirs[1].VirtualAddress) == 0 || EC32(peinfo->dirs[1].Size) == 0) {
          cli_dbgmsg("scan_pe: import table data dir does not exist (skipping .imp scanning)\n");
          status = CL_BREAK;
          goto done;
      }

-    // TODO Add EC32 wrappers
-    impoff = cli_rawaddr(peinfo->dirs[1].VirtualAddress, peinfo->sections, peinfo->nsections, &err, fsize, peinfo->hdr_size);
-    if (err || impoff + peinfo->dirs[1].Size > fsize) {
+    impoff = cli_rawaddr(EC32(peinfo->dirs[1].VirtualAddress), peinfo->sections, peinfo->nsections, &err, fsize, peinfo->hdr_size);
+    if (err || impoff + EC32(peinfo->dirs[1].Size) > fsize) {
          cli_dbgmsg("scan_pe: invalid rva for import table data\n");
          status = CL_BREAK;
          goto done;
      }

-    // TODO Add EC32 wrapper
-    impdes = (const struct pe_image_import_descriptor *)fmap_need_off(map, impoff, peinfo->dirs[1].Size);
+    impdes = (const struct pe_image_import_descriptor *)fmap_need_off(map, impoff, EC32(peinfo->dirs[1].Size));
      if (impdes == NULL) {
          cli_dbgmsg("scan_pe: failed to acquire fmap buffer\n");
          status = CL_EREAD;
@@ -2447,7 +2445,7 @@ static cl_error_t hash_imptbl(cli_ctx *c

      /* Safety: We can trust peinfo->dirs[1].Size only because `fmap_need_off()` (above)
       * would have failed if the size exceeds the end of the fmap. */
-    left = peinfo->dirs[1].Size;
+    left = EC32(peinfo->dirs[1].Size);

      if (genhash[CLI_HASH_MD5]) {
          hashctx[CLI_HASH_MD5] = cl_hash_init("md5");
@@ -2546,7 +2544,7 @@ static cl_error_t hash_imptbl(cli_ctx *c

  done:
      if (needed_impoff) {
-        fmap_unneed_off(map, impoff, peinfo->dirs[1].Size);
+        fmap_unneed_off(map, impoff, EC32(peinfo->dirs[1].Size));
      }

      for (type = CLI_HASH_MD5; type < CLI_HASH_AVAIL_TYPES; type++) {
@@ -3177,8 +3175,7 @@ int cli_scanpe(cli_ctx *ctx)
      }

      /* W32.Polipos.A */
-    // TODO Add endianness correction to SizeOfStackReserve access
-    while (polipos && !peinfo->is_dll && peinfo->nsections > 2 && peinfo->nsections < 13 && peinfo->e_lfanew <= 0x800 && (EC16(peinfo->pe_opt.opt32.Subsystem) == 2 || EC16(peinfo->pe_opt.opt32.Subsystem) == 3) && EC16(peinfo->file_hdr.Machine) == 0x14c && peinfo->pe_opt.opt32.SizeOfStackReserve >= 0x80000) {
+    while (polipos && !peinfo->is_dll && peinfo->nsections > 2 && peinfo->nsections < 13 && peinfo->e_lfanew <= 0x800 && (EC16(peinfo->pe_opt.opt32.Subsystem) == 2 || EC16(peinfo->pe_opt.opt32.Subsystem) == 3) && EC16(peinfo->file_hdr.Machine) == 0x14c && EC32(peinfo->pe_opt.opt32.SizeOfStackReserve) >= 0x80000) {
          uint32_t jump, jold, *jumps = NULL;
          const uint8_t *code;
          unsigned int xsjs = 0;
@@ -3250,7 +3247,7 @@ int cli_scanpe(cli_ctx *ctx)

      /* Trojan.Swizzor.Gen */
      if (SCAN_HEURISTICS && (DCONF & PE_CONF_SWIZZOR) && peinfo->nsections > 1 && fsize > 64 * 1024 && fsize < 4 * 1024 * 1024) {
-        if (peinfo->dirs[2].Size) {
+        if (EC32(peinfo->dirs[2].Size)) {
              struct swizz_stats *stats = cli_calloc(1, sizeof(*stats));
              unsigned int m            = 1000;
              ret                       = CL_CLEAN;
@@ -3903,8 +3900,7 @@ int cli_scanpe(cli_ctx *ctx)
          if (cli_memstr(UPX_LZMA2, 20, epbuff + 0x2f, 20)) {
              uint32_t strictdsize = cli_readint32(epbuff + 0x21), skew = 0;
              if (ssize > 0x15 && epbuff[0] == '\x60' && epbuff[1] == '\xbe') {
-                // TODO Add EC32
-                skew = cli_readint32(epbuff + 2) - peinfo->sections[i + 1].rva - peinfo->pe_opt.opt32.ImageBase;
+                skew = cli_readint32(epbuff + 2) - peinfo->sections[i + 1].rva - EC32(peinfo->pe_opt.opt32.ImageBase);
                  if (skew != 0x15)
                      skew = 0;
              }
@@ -3915,8 +3911,7 @@ int cli_scanpe(cli_ctx *ctx)
              uint32_t strictdsize = cli_readint32(epbuff + 0x2b), skew = 0;
              uint32_t properties = cli_readint32(epbuff + 0x41);
              if (ssize > 0x15 && epbuff[0] == '\x60' && epbuff[1] == '\xbe') {
-                // TODO Add EC32
-                skew = cli_readint32(epbuff + 2) - peinfo->sections[i + 1].rva - peinfo->pe_opt.opt32.ImageBase;
+                skew = cli_readint32(epbuff + 2) - peinfo->sections[i + 1].rva - EC32(peinfo->pe_opt.opt32.ImageBase);
                  if (skew != 0x15)
                      skew = 0;
              }
@@ -5097,7 +5092,7 @@ cl_error_t cli_peheader(fmap_t *map, str

      for (i = 0; falign != 0x200 && i < (size_t)peinfo->nsections; i++) {
          /* file alignment fallback mode - blah */
-        if (falign && section_hdrs[i].SizeOfRawData && EC32(section_hdrs[i].PointerToRawData) % falign && !(EC32(section_hdrs[i].PointerToRawData) % 0x200)) {
+        if (falign && EC32(section_hdrs[i].SizeOfRawData) && EC32(section_hdrs[i].PointerToRawData) % falign && !(EC32(section_hdrs[i].PointerToRawData) % 0x200)) {
              cli_dbgmsg("cli_peheader: Encountered section with unexpected alignment - triggering fallback mode\n");
              falign = 0x200;
          }
@@ -5292,13 +5287,13 @@ cl_error_t cli_peheader(fmap_t *map, str
          cli_dbgmsg("EntryPoint offset: 0x%x (%d)\n", peinfo->ep, peinfo->ep);
      }

-    if (is_dll || peinfo->ndatadirs < 3 || !peinfo->dirs[2].Size)
+    if (is_dll || peinfo->ndatadirs < 3 || !EC32(peinfo->dirs[2].Size))
          peinfo->res_addr = 0;
      else
          peinfo->res_addr = EC32(peinfo->dirs[2].VirtualAddress);

      while (opts & CLI_PEHEADER_OPT_EXTRACT_VINFO &&
-           peinfo->ndatadirs >= 3 && peinfo->dirs[2].Size) {
+           peinfo->ndatadirs >= 3 && EC32(peinfo->dirs[2].Size)) {
          struct vinfo_list vlist;
          const uint8_t *vptr, *baseptr;
          uint32_t rva, res_sz;


-- 
SUSE Software Solutions Germany GmbH (HRB 36809, AG Nürnberg)
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman


More information about the clamav-devel mailing list