[PATCH rebase 1/2] Always update the file checksum in the PE header

[PATCH rebase 1/2] Always update the file checksum in the PE header

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 116 bytes --]

This patch is still experimental, but tested with all /bin/cyg*.dll from 
my installation.

-- 
Regards,
Christian


[-- Attachment #2: 0001-Always-update-the-file-checksum-in-the-PE-header.patch --]
[-- Type: text/plain, Size: 25884 bytes --]

From 53663d46c2c989e665143b33c0614b416fd1c666 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Sat, 12 Aug 2023 14:13:43 +0200
Subject: [PATCH] Always update the file checksum in the PE header

Both peflags and rebase now update the checksum without reading
the whole file under the assumption that the original checksum
was correct.  Calculation is done by new module
'imagehelper/pechecksum_update.c'.
Slightly change the peflags -k, --checksum option accordingly.

Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
 Makefile.in                     |   6 +-
 imagehelper/Makefile.in         |   7 ++-
 imagehelper/imagehelper.h       |   4 ++
 imagehelper/objectfile.cc       |  62 +++++++++++++++---
 imagehelper/objectfile.h        |   7 +++
 imagehelper/pechecksum_update.c |  56 +++++++++++++++++
 imagehelper/pechecksum_update.h |  52 ++++++++++++++++
 imagehelper/rebaseimage.cc      |   4 ++
 imagehelper/sections.cc         |  20 +++++-
 imagehelper/sections.h          |   8 ++-
 peflags.c                       | 107 +++++++++++++++++++++-----------
 rebase.c                        |  39 ++++++++----
 12 files changed, 305 insertions(+), 67 deletions(-)
 create mode 100644 imagehelper/pechecksum_update.c
 create mode 100644 imagehelper/pechecksum_update.h

diff --git a/Makefile.in b/Makefile.in
index e7b7f6a..edc9eda 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -81,7 +81,7 @@ REBASE_DUMP_OBJS = rebase-dump.$(O) rebase-db.$(O) $(LIBOBJS)
 REBASE_DUMP_LIBS =
 
 PEFLAGS_OBJS = peflags.$(O) pechecksum.$(O) $(LIBOBJS)
-PEFLAGS_LIBS =
+PEFLAGS_LIBS = $(LIBIMAGEHELPER)
 
 SRC_DISTFILES = configure.ac configure Makefile.in \
 	peflagsall.in rebaseall.in \
@@ -109,8 +109,8 @@ rebase-dump$(EXEEXT): $(REBASE_DUMP_OBJS)
 
 rebase-dump.$(O):: rebase-dump.c rebase-db.h Makefile
 
-peflags$(EXEEXT): $(PEFLAGS_OBJS)
-	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(PEFLAGS_OBJS)
+peflags$(EXEEXT): $(PEFLAGS_OBJS) $(PEFLAGS_LIBS)
+	$(CXX) $(CXXFLAGS) $(LDFLAGS) $(CXX_LDFLAGS) -o $@ $(PEFLAGS_OBJS) $(PEFLAGS_LIBS)
 
 peflags.$(O):: peflags.c pechecksum.h Makefile
 
diff --git a/imagehelper/Makefile.in b/imagehelper/Makefile.in
index 0ed8e6c..a40cd8c 100755
--- a/imagehelper/Makefile.in
+++ b/imagehelper/Makefile.in
@@ -75,11 +75,12 @@ LIB_TARGET=imagehelper
 LIB_TARGET_FILE=libimagehelper.a
 LIB_OBJS = objectfile.$(O) objectfilelist.$(O) sections.$(O) debug.$(O) \
 	rebaseimage.$(O) checkimage.$(O) fiximage.$(O) getimageinfos.$(O) \
-	bindimage.$(O)
+	bindimage.$(O) pechecksum_update.$(O)
 LIB_SRCS = objectfile.cc objectfilelist.cc sections.cc debug.cc \
 	rebaseimage.cc checkimage.cc fiximage.cc getimageinfos.cc \
-	bindimage.cc
-LIB_HDRS = objectfilelist.h imagehelper.h sections.h objectfile.h
+	bindimage.cc pechecksum_update.c
+LIB_HDRS = objectfilelist.h imagehelper.h sections.h objectfile.h \
+	pechecksum_update.h
 
 #
 # (obsolete) applications
diff --git a/imagehelper/imagehelper.h b/imagehelper/imagehelper.h
index bafc106..8535311 100755
--- a/imagehelper/imagehelper.h
+++ b/imagehelper/imagehelper.h
@@ -33,6 +33,10 @@ extern BOOL ReBaseChangeFileTime;
 /* Set to TRUE, if rebasing should also drop the /DYNAMICBASE flag
    from the PE flags. */
 extern BOOL ReBaseDropDynamicbaseFlag;
+/* Set to TRUE if ReBaseImage{64} should update the PE checksum.
+   Cleared if the update failed due to inconsistent original
+   checksum. */
+extern BOOL ReBaseUpdateCheckSum;
 
 BOOL ReBaseImage64(
   LPCSTR CurrentImageName,
diff --git a/imagehelper/objectfile.cc b/imagehelper/objectfile.cc
index 4dafa6a..651fcce 100755
--- a/imagehelper/objectfile.cc
+++ b/imagehelper/objectfile.cc
@@ -24,6 +24,7 @@
 #include <time.h>
 
 #include "objectfile.h"
+#include "pechecksum_update.h"
 
 // read a dll into the cache
 
@@ -109,6 +110,15 @@ ObjectFile::ObjectFile(const char *aFileName, bool writeable)
       FileName = strdup(name);
     }
 
+  LARGE_INTEGER fs;
+  if (!GetFileSizeEx(hfile, &fs))
+    {
+      CloseHandle(hfile);
+      Error = 2;
+      return;
+    }
+  FileSize = fs.QuadPart;
+
   hfilemapping = CreateFileMapping(hfile, NULL, writeable ? PAGE_READWRITE : PAGE_READONLY , 0, 0,  NULL);
   if (hfilemapping == 0)
     {
@@ -139,8 +149,7 @@ ObjectFile::ObjectFile(const char *aFileName, bool writeable)
       return;
     }
   // filesize big enough to allow at least reading the NT header?
-  if (GetFileSize (hfile, NULL)
-      < (char *) ntheader - (char *) dosheader + sizeof *ntheader)
+  if (FileSize < (char *) ntheader - (char *) dosheader + sizeof *ntheader)
     {
       Error = 4;
       return;
@@ -178,9 +187,6 @@ ObjectFile::~ObjectFile()
     CloseHandle(hfile);
 }
 
-
-
-
 int LinkedObjectFile::level = 0;
 
 LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) : ObjectFile(aFileName,writable)
@@ -204,6 +210,12 @@ LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) : Objec
   PIMAGE_NT_HEADERS32 ntheader32 = getNTHeader32 ();
   PIMAGE_NT_HEADERS64 ntheader64 = getNTHeader64 ();
 
+  // Save original header for checksum update
+  if (is64bit ())
+    old_ntheader64 = *ntheader64;
+  else
+    old_ntheader32 = *ntheader32;
+
   Section *edata = sections->find(".edata");
   if (edata)
     exports = new Exports(*edata);
@@ -231,9 +243,13 @@ LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) : Objec
       imports = new Imports(*sections, dir);
     }
 
+  uint32_t checksum;
+  if (is64bit ())
+    checksum = getNTHeader64 ()->OptionalHeader.CheckSum;
+  else
+    checksum = getNTHeader32 ()->OptionalHeader.CheckSum;
 
-
-  relocs = new Relocations(*sections,".reloc");
+  relocs = new Relocations(*sections, ".reloc", pe32_checksum_update_begin(checksum, FileSize));
 
 }
 
@@ -474,6 +490,38 @@ bool LinkedObjectFile::unbind(void)
   return true;
 }
 
+BOOL LinkedObjectFile::updateCheckSum()
+{
+  ULONG *p_checksum;
+  if (is64bit ())
+    p_checksum = &getNTHeader64 ()->OptionalHeader.CheckSum;
+  else
+    p_checksum = &getNTHeader32 ()->OptionalHeader.CheckSum;
+
+  // Add changes from NT header to checksum updated by relocs
+  uint16_t checksum16;
+  if (relocs)
+    checksum16 = relocs->getCheckSum16();
+  else
+    checksum16 = pe32_checksum_update_begin(*p_checksum, FileSize);
+
+  if (is64bit ())
+    checksum16 = pe32_checksum_update_add(checksum16, &old_ntheader64,
+					  getNTHeader64 (),
+					  sizeof(old_ntheader64));
+  else
+    checksum16 = pe32_checksum_update_add(checksum16, &old_ntheader32,
+					 getNTHeader32 (),
+					 sizeof(old_ntheader32));
+
+  uint32_t checksum = pe32_checksum_update_end(checksum16, FileSize);
+  if (!checksum)
+    return FALSE;
+
+  // Update the PE header
+  *p_checksum = checksum;
+  return TRUE;
+}
 
 LinkedObjectFile::~LinkedObjectFile()
 {
diff --git a/imagehelper/objectfile.h b/imagehelper/objectfile.h
index cd9509c..9deda1f 100755
--- a/imagehelper/objectfile.h
+++ b/imagehelper/objectfile.h
@@ -92,6 +92,7 @@ class ObjectFile : public Base
 
   protected:
     char *FileName;
+    ULONG64 FileSize;
     HANDLE hfile;
     HANDLE hfilemapping;
     LPVOID lpFileBase;
@@ -140,12 +141,18 @@ class LinkedObjectFile : public ObjectFile
       return exports;
     }
 
+    BOOL updateCheckSum();
+
   protected:
     Imports *imports;
     Exports *exports;
     Relocations *relocs;
     static int level;
     bool isPrinted;
+    union {
+      IMAGE_NT_HEADERS32 old_ntheader32;
+      IMAGE_NT_HEADERS64 old_ntheader64;
+    };
   };
 
 #include "objectfilelist.h"
diff --git a/imagehelper/pechecksum_update.c b/imagehelper/pechecksum_update.c
new file mode 100644
index 0000000..c15579d
--- /dev/null
+++ b/imagehelper/pechecksum_update.c
@@ -0,0 +1,56 @@
+/*
+ * PE32 checksum update
+ *
+ * Copyright (C) 2023 Christian Franke
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include "pechecksum_update.h"
+
+uint16_t pe32_checksum_update_begin(uint32_t checksum, unsigned long long filesize)
+{
+  if (!(0 < checksum && filesize < checksum && checksum - filesize <= 0xffffULL))
+    return 0;
+  return (uint16_t)(checksum - filesize);
+}
+
+static inline uint16_t get_word(const void * p, unsigned i)
+{
+  const uint8_t * bp = p;
+  return bp[i] | (bp[i + 1] << 8);
+}
+
+uint16_t pe32_checksum_update_add(uint16_t checksum16, const void * old_block,
+                                  const void * new_block, unsigned size)
+{
+  if (!(checksum16 && !((uintptr_t)old_block & 1)
+        && !((uintptr_t)new_block & 1) && !(size & 1)))
+    return 0;
+  uint32_t sum = checksum16;
+
+  for (unsigned i = 0; i < size; i += 2) {
+    uint16_t old_word = get_word(old_block, i);
+    uint16_t new_word = get_word(new_block, i);
+    if (new_word == old_word)
+      continue;
+    // Note: This does not work for updates of the first nonzero word of the file.
+    // This does not matter because the first word "MZ" is never changed.
+    if (old_word < sum) // Never return to 0, so no '<='.
+      sum -= old_word;
+    else // Revert previous 16-bit overflow, skip 0.
+      sum += 0xffffU - old_word;
+    sum += new_word;
+    // If 16-bit overflow, skip 0.
+    sum = (sum + (sum >> 16)) & 0xffffU;
+  }
+
+  return (uint16_t)sum;
+}
+
+uint32_t pe32_checksum_update_end(uint16_t checksum16, unsigned long long filesize)
+{
+  if (!(checksum16 && checksum16 + filesize < 0xffffffffULL))
+    return 0;
+  return (uint32_t)(checksum16 + filesize);
+}
diff --git a/imagehelper/pechecksum_update.h b/imagehelper/pechecksum_update.h
new file mode 100644
index 0000000..7a2c911
--- /dev/null
+++ b/imagehelper/pechecksum_update.h
@@ -0,0 +1,52 @@
+/*
+ * PE32 checksum update
+ *
+ * Copyright (C) 2023 Christian Franke
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#ifndef PE_CHECKSUM_UPDATE_H
+#define PE_CHECKSUM_UPDATE_H
+
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/*
+ * Begin checksum update sequence.
+ * checksum: Old 32-bit checksum from PE header.
+ * filesize: Size of PE file.
+ * return value: 16-bit checksum for p32_checksum_update_add()
+ * or 0 on error (the checksum itself is never 0).
+ */
+uint16_t pe32_checksum_update_begin(uint32_t checksum, unsigned long long filesize);
+
+/*
+ * Update checksum.
+ * checksum16: Current 16-bit checksum from p32_checksum_update_begin()
+ * or this function.
+ * old_block: Address of old (original) data block, must be word aligned.
+ * new_block: Address of new (changed) data block, must be word aligned.
+ * size: Number of bytes of both data blocks, must be even.
+ * The PE checksum field may be included if identical in both blocks.
+ * return value: Updated 16-bit checksum or 0 on error.
+ */
+uint16_t pe32_checksum_update_add(uint16_t checksum16, const void * old_block,
+                                  const void * new_block, unsigned size);
+
+/*
+ * End checksum update sequence.
+ * checksum16: New 16-bit checksum from pe32_checksum_update_add().
+ * filesize: Size of PE file.
+ * return value: New 32-bit checksum for PE header or 0 on error.
+ */
+uint32_t pe32_checksum_update_end(uint16_t checksum16, unsigned long long filesize);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* PE_CHECKSUM_UPDATE_H */
diff --git a/imagehelper/rebaseimage.cc b/imagehelper/rebaseimage.cc
index 2f1fb03..8827a14 100755
--- a/imagehelper/rebaseimage.cc
+++ b/imagehelper/rebaseimage.cc
@@ -32,6 +32,7 @@
 
 BOOL ReBaseChangeFileTime = FALSE;
 BOOL ReBaseDropDynamicbaseFlag = FALSE;
+BOOL ReBaseUpdateCheckSum = FALSE;
 
 BOOL ReBaseImage64 (
   LPCSTR CurrentImageName,
@@ -141,6 +142,9 @@ BOOL ReBaseImage64 (
 	  &= ~IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE;
     }
 
+  if (ReBaseUpdateCheckSum && !dll.updateCheckSum())
+    ReBaseUpdateCheckSum = FALSE;
+
   if (!fGoingDown)
     *NewImageBase += *NewImageSize;
 
diff --git a/imagehelper/sections.cc b/imagehelper/sections.cc
index dbf7da0..76f46ee 100755
--- a/imagehelper/sections.cc
+++ b/imagehelper/sections.cc
@@ -22,8 +22,10 @@
 
 #include <iostream>
 #include <iomanip>
+#include <string.h>
 
 #include "sections.h"
+#include "pechecksum_update.h"
 
 int Base::debug = 0;
 
@@ -270,10 +272,11 @@ void Imports::dump(const char *title)
 //----- class Imports --------------------------------------------------
 
 
-Relocations::Relocations(SectionList &sectionList, const char *sectionName)
+Relocations::Relocations(SectionList &sectionList, const char *sectionName, uint16_t checksum16_)
 {
   sections = &sectionList;
   Section *asection = sections->find(sectionName);
+  checksum16 = checksum16_;
   if (asection)
     {
       relocs = PIMAGE_BASE_RELOCATION (asection->getStartAddress());
@@ -363,8 +366,11 @@ bool Relocations::fix(void)
         std::cerr << "no errors found" << std::endl;
     }
   else
-    if (debug)
-      std::cerr << "corrupted relocation records fixed" << std::endl;
+    {
+      checksum16 = 0; // Checksum update not implemented.
+      if (debug)
+	std::cerr << "corrupted relocation records fixed" << std::endl;
+    }
   return true;
 }
 
@@ -425,7 +431,12 @@ bool Relocations::relocate(int64_t difference)
 		    << std::endl;
 		  }
 		int32_t *patch_adr = (int *)cursec->rva2real(location);
+		unsigned cs_offs = (uintptr_t)patch_adr & 1; // 32-bit relocations may be not word aligned.
+		const uint8_t *cs_adr = (const uint8_t *)patch_adr - cs_offs;
+		unsigned cs_size = sizeof(uint32_t) + cs_offs * 2;
+		uint8_t old_val[cs_size]; memcpy(old_val, cs_adr, cs_size);
 		*patch_adr += difference;
+		checksum16 = pe32_checksum_update_add(checksum16, old_val, cs_adr, cs_size);
 	      }
 	      break;
 	    case IMAGE_REL_BASED_DIR64:
@@ -440,7 +451,10 @@ bool Relocations::relocate(int64_t difference)
 		    << std::endl;
 		  }
 		int64_t *patch_adr = (int64_t *)cursec->rva2real(location);
+		int64_t old_val = *patch_adr;
 		*patch_adr += difference;
+		// Assume that 64-bit relocations are always word aligned.
+		checksum16 = pe32_checksum_update_add(checksum16, &old_val, patch_adr, sizeof(old_val));
 	      }
 	      break;
 	    default:
diff --git a/imagehelper/sections.h b/imagehelper/sections.h
index b317fa1..20dd24c 100755
--- a/imagehelper/sections.h
+++ b/imagehelper/sections.h
@@ -199,7 +199,7 @@ class Relocations : SectionBase
   {
   public:
     // create a relocation object using section named "section" from  the section list "sections"
-    Relocations(SectionList &sectionList, const char *sectionName);
+    Relocations(SectionList &sectionList, const char *sectionName, uint16_t checksum16_);
 
     // check for bad relocations
     bool check(void);
@@ -210,10 +210,16 @@ class Relocations : SectionBase
     // precondition: fixed dll
     bool relocate(int64_t difference);
 
+    uint16_t getCheckSum16() const
+      {
+	return checksum16;
+      }
+
   private:
     PIMAGE_BASE_RELOCATION relocs;
     SectionList *sections;
     int size;   // section size
+    uint16_t checksum16;
   };
 
 #endif
diff --git a/peflags.c b/peflags.c
index 1a61da7..917088b 100644
--- a/peflags.c
+++ b/peflags.c
@@ -44,6 +44,7 @@
 #include <windows.h>
 
 #include "pechecksum.h"
+#include "imagehelper/pechecksum_update.h"
 
 #if defined(__MSYS__)
 /* MSYS has no strtoull */
@@ -67,12 +68,12 @@ static WORD coff_characteristics_show;
 static WORD pe_characteristics_set;
 static WORD pe_characteristics_clr;
 static WORD pe_characteristics_show;
-static short checksum_show;
-static short checksum_update; /* 1 = update after changes, 2 = fix always */
+static short checksum_option; /* 1 = show, 2 = verify, 3 = fix */
 
 typedef struct
 {
   const char *pathname;
+  unsigned long long filesize;
   PIMAGE_DOS_HEADER dosheader;
   union
     {
@@ -507,24 +508,48 @@ do_mark (const char *pathname)
 
     }
 
-  /* Any actual changes? */
-  if (pep->is_64bit)
-    {
-      changed = memcmp(&old_ntheader.ntheader64, pep->ntheader64,
-		       sizeof(old_ntheader.ntheader64));
-      hdr_checksum = pep->ntheader64->OptionalHeader.CheckSum;
-    }
-  else
-    {
-      changed = memcmp(&old_ntheader.ntheader32, pep->ntheader64,
-		       sizeof(old_ntheader.ntheader32));
-      hdr_checksum = pep->ntheader32->OptionalHeader.CheckSum;
-    }
+  /* Update the checksum. */
+  changed = FALSE;
+  {
+    const void *p_oldheader, *p_newheader;
+    unsigned headersize;
+    ULONG *p_checksum;
+    if (pep->is_64bit)
+      {
+	p_oldheader = &old_ntheader.ntheader64;
+	p_newheader = pep->ntheader64;
+	headersize = sizeof(old_ntheader.ntheader64);
+	p_checksum = &pep->ntheader64->OptionalHeader.CheckSum;
+      }
+    else
+      {
+	p_oldheader = &old_ntheader.ntheader32;
+	p_newheader = pep->ntheader32;
+	headersize = sizeof(old_ntheader.ntheader32);
+	p_checksum = &pep->ntheader32->OptionalHeader.CheckSum;
+      }
+
+    hdr_checksum = pe32_checksum_update_end(
+      pe32_checksum_update_add(
+	pe32_checksum_update_begin(*p_checksum, pep->filesize),
+	p_oldheader, p_newheader, headersize
+      ),
+      pep->filesize
+    );
+
+    if (!hdr_checksum)
+      fprintf(stderr, "%s: Checksum update failed\n", pathname);
+    else if (*p_checksum != hdr_checksum)
+      {
+	changed = TRUE;
+	*p_checksum = hdr_checksum;
+      }
+  }
 
   pe_close (pep);
 
-  /* Checksum calculation requires re-open as a regular file. */
-  if (checksum_show || checksum_update)
+  /* Checksum re-calculation requires re-open as a regular file. */
+  if (checksum_option)
     do_checksum (pathname, indent, changed, hdr_checksum);
   return 0;
 }
@@ -625,16 +650,16 @@ static void
 do_checksum (const char *pathname, int indent, int changed, ULONG hdr_checksum)
 {
   const char name[] = "PE file checksum";
-  if (checksum_show || (checksum_update && changed) || checksum_update > 1)
+  if (checksum_option > 1)
     {
       int fd;
       unsigned new_checksum, old_checksum;
-      if ((fd = open (pathname, (checksum_update ? O_RDWR : O_RDONLY) | O_BINARY)) == -1)
+      if ((fd = open (pathname, (checksum_option > 2 ? O_RDWR : O_RDONLY) | O_BINARY)) == -1)
 	{
 	  fprintf (stderr, "%s: Unable to reopen, errno=%d\n", pathname, errno);
 	  return;
 	}
-      new_checksum = pe32_checksum (fd, checksum_update, &old_checksum);
+      new_checksum = pe32_checksum (fd, checksum_option > 2, &old_checksum);
       close(fd);
       if (!new_checksum)
 	{
@@ -650,19 +675,19 @@ do_checksum (const char *pathname, int indent, int changed, ULONG hdr_checksum)
 	}
 
       if (new_checksum == old_checksum)
-	  printf ("%*s%-24s: 0x%08x (%sverified)\n", indent, "", name,
-		  old_checksum, (checksum_update ? "unchanged, " : ""));
-      else if (checksum_update)
+	  printf ("%*s%-24s: 0x%08x (%schanged, verified)\n", indent, "", name,
+	          old_checksum, (changed ? "" : "un"));
+      else if (checksum_option > 2)
 	  printf ("%*s%-24s: 0x%08x (updated from 0x%08x)\n", indent, "", name,
 		  new_checksum, old_checksum);
       else
 	  printf ("%*s%-24s: 0x%08x (*needs update to 0x%08x*)\n", indent, "", name,
 		  old_checksum, new_checksum);
     }
-  else /* (!checksum_show && checksum_update == 1 && !changed) */
+  else /* (checksum_option == 1) */
     {
-      printf ("%*s%-24s: 0x%08x (unchanged, not verified)\n", indent, "", name,
-	      hdr_checksum);
+      printf ("%*s%-24s: 0x%08x (%schanged, not verified)\n", indent, "", name,
+	      hdr_checksum, (changed ? "" : "un"));
     }
 }
 
@@ -767,11 +792,7 @@ handle_checksum_option (const char *option_name,
 {
   int bool_value;
   if (!option_arg)
-    {
-      checksum_show = 1;
-      if (handle_any_sizeof == DONT_HANDLE)
-	handle_any_sizeof = DO_READ;
-    }
+    checksum_option = 1;
   else
     {
       if (string_to_bool (option_arg, &bool_value) != 0)
@@ -781,9 +802,12 @@ handle_checksum_option (const char *option_name,
           short_usage (stderr);
           exit (1);
         }
-      checksum_update = (bool_value ? 2 : 1);
-      handle_any_sizeof = DO_WRITE;
+      checksum_option = (!bool_value ? 2 : 3);
     }
+  if (checksum_option > 2)
+    handle_any_sizeof = DO_WRITE;
+  else if (handle_any_sizeof == DONT_HANDLE)
+    handle_any_sizeof = DO_READ;
 }
 
 void
@@ -1042,6 +1066,11 @@ pe_open (const char *path, BOOL writing)
   int fd;
   void *map;
   
+  /* Get filesize for checksum update. */
+  struct stat st;
+  if (stat (path, &st))
+    return NULL;
+
   fd = open (path, O_BINARY | (writing ? O_RDWR : O_RDONLY));
   if (fd == -1)
     return NULL;
@@ -1053,6 +1082,7 @@ pe_open (const char *path, BOOL writing)
       return NULL;
     }
   pef.pathname = path;
+  pef.filesize = st.st_size;
   pef.dosheader = (PIMAGE_DOS_HEADER) map;
   pef.ntheader32 = (PIMAGE_NT_HEADERS32)
 		   ((PBYTE) pef.dosheader + pef.dosheader->e_lfanew);
@@ -1233,9 +1263,12 @@ help (FILE *f)
 "                              than 2 GB.\n"
 "  -S, --sepdbg       [BOOL]   Debugging information was removed and stored\n"
 "                              separately in another file.\n"
-"  -k, --checksum     [BOOL]   Displays (no argument), updates after changes\n"
-"                              only (false) or always fixes (true) the file\n"
-"                              checksum in the PE header.\n"
+"  -k, --checksum     [BOOL]   Displays (no argument), verifies (false) or\n"
+"                              always fixes (true) the file checksum in the PE\n"
+"                              header.  By default, the checksum is updated\n"
+"                              without reading the whole file under the\n"
+"                              assumption that the original checksum was\n"
+"                              correct.\n"
 "  -x, --stack-reserve [NUM]   Reserved stack size of the process in bytes.\n"
 "  -X, --stack-commit  [NUM]   Initial commited portion of the process stack\n"
 "                              in bytes.\n"
@@ -1272,7 +1305,7 @@ help (FILE *f)
 "For flag values, to set a value, and display the results symbolic, repeat the\n"
 "option:  --tsaware=true --tsaware -d0 -d\n"
 "The time of last modification of each file is preserved unless the checksum\n"
-"has been updated.\n"
+"has been fixed by option --checksum=true\n"
 "\n", f);
 }
 
diff --git a/rebase.c b/rebase.c
index 50cc79f..20a9902 100644
--- a/rebase.c
+++ b/rebase.c
@@ -1190,7 +1190,7 @@ print_overlapped ()
     }
 }
 
-static BOOL
+static int
 update_checksum (const char *pathname)
 {
   int fd, err;
@@ -1200,7 +1200,7 @@ update_checksum (const char *pathname)
     {
       fprintf (stderr, "%s: failed to reopen for checksum update: %s\n",
 	       pathname, strerror (errno));
-      return FALSE;
+      return -1;
     }
   new_checksum = pe32_checksum (fd, 1, &old_checksum);
   err = errno;
@@ -1210,10 +1210,10 @@ update_checksum (const char *pathname)
       fprintf (stderr, "%s: checksum update failed: %s\n", pathname,
 	       strerror (err));
       /* Assume file is unchanged. */
-      return FALSE;
+      return -1;
     }
 
-  return (new_checksum != old_checksum);
+  return (new_checksum != old_checksum ? 1 : 0);
 }
 
 BOOL
@@ -1222,6 +1222,7 @@ rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag)
   ULONG64 old_image_base, prev_new_image_base;
   ULONG old_image_size, new_image_size;
   BOOL status;
+  const char *checksum_msg;
 
   /* Skip if not writable. */
   if (access (pathname, W_OK) == -1)
@@ -1231,6 +1232,9 @@ rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag)
       return TRUE;
     }
 
+  /* Try to update the checksum. */
+  ReBaseUpdateCheckSum = TRUE;
+
   /* Calculate next base address, if rebasing down. */
   if (down_flag)
     *new_image_base -= offset;
@@ -1307,8 +1311,17 @@ retry:
     }
 #endif
 
-  /* Update checksum, if requested. */
-  status = (checksum_flag ? update_checksum (pathname) : FALSE);
+
+  /* Fix checksum, if requested or incremental update failed. */
+  if (checksum_flag || !ReBaseUpdateCheckSum)
+    switch (update_checksum (pathname))
+      {
+	case 0:  checksum_msg = ", checksum verified"; break;
+	case 1:  checksum_msg = ", checksum fixed"; break;
+	default: checksum_msg = ", checksum update failed"; break;
+      }
+  else
+    checksum_msg = "";
 
   /* Display rebase results, if verbose. */
   if (verbose)
@@ -1316,9 +1329,7 @@ retry:
       printf ("%s: new base = %" PRIx64 ", new size = %x%s\n",
 	      pathname,
 	      (uint64_t) ((down_flag) ? *new_image_base : prev_new_image_base),
-	      (uint32_t) (new_image_size + offset),
-	      (checksum_flag ? (status ? ", checksum updated" :
-	      ", checksum unchanged") : ""));
+	      (uint32_t) (new_image_size + offset), checksum_msg);
     }
 
   /* Calculate next base address, if rebasing up. */
@@ -1662,10 +1673,12 @@ Rebase PE files, usually DLLs, to a specified address or address range.\n\
   One of the options -b, -s or -i is mandatory.  If no rebase database exists\n\
   yet, -b is required together with -s.\n\
 \n\
-  -c, --checksum          Update the file's checksum in the PE header if the\n\
-                          file has been successfully rebased.  This also bumps\n\
-                          the file's modification time (like -t) if the\n\
-                          checksum has been changed.\n\
+  -c, --checksum          Fix the file's checksum in the PE header if the file\n\
+                          has been successfully rebased.  This also bumps the\n\
+                          file's modification time (like -t) if the checksum\n\
+                          has been changed.  By default, the checksum is\n\
+                          updated without reading the whole file under the\n\
+                          assumption that the original checksum was correct.\n\
   -d, --down              Treat the BaseAddress as upper ceiling and rebase\n\
                           files top-down from there.  Without this option the\n\
                           files are rebased from BaseAddress bottom-up.\n\
-- 
2.39.0

Re: [PATCH rebase 1/2] Always update the file checksum in the PE header

[Corinna Vinschen]

On Aug 12 14:26, Christian Franke via Cygwin-apps wrote:
> This patch is still experimental, but tested with all /bin/cyg*.dll from my
> installation.

Does that mean I shouldn't apply it for now, or do you want me to
apply it but not create a new release yet?


Thanks,
Corinna


> 
> -- 
> Regards,
> Christian
> 

> From 53663d46c2c989e665143b33c0614b416fd1c666 Mon Sep 17 00:00:00 2001
> From: Christian Franke <christian.franke@t-online.de>
> Date: Sat, 12 Aug 2023 14:13:43 +0200
> Subject: [PATCH] Always update the file checksum in the PE header
> 
> Both peflags and rebase now update the checksum without reading
> the whole file under the assumption that the original checksum
> was correct.  Calculation is done by new module
> 'imagehelper/pechecksum_update.c'.
> Slightly change the peflags -k, --checksum option accordingly.
> 
> Signed-off-by: Christian Franke <christian.franke@t-online.de>
> ---
>  Makefile.in                     |   6 +-
>  imagehelper/Makefile.in         |   7 ++-
>  imagehelper/imagehelper.h       |   4 ++
>  imagehelper/objectfile.cc       |  62 +++++++++++++++---
>  imagehelper/objectfile.h        |   7 +++
>  imagehelper/pechecksum_update.c |  56 +++++++++++++++++
>  imagehelper/pechecksum_update.h |  52 ++++++++++++++++
>  imagehelper/rebaseimage.cc      |   4 ++
>  imagehelper/sections.cc         |  20 +++++-
>  imagehelper/sections.h          |   8 ++-
>  peflags.c                       | 107 +++++++++++++++++++++-----------
>  rebase.c                        |  39 ++++++++----
>  12 files changed, 305 insertions(+), 67 deletions(-)
>  create mode 100644 imagehelper/pechecksum_update.c
>  create mode 100644 imagehelper/pechecksum_update.h
> 
> diff --git a/Makefile.in b/Makefile.in
> index e7b7f6a..edc9eda 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -81,7 +81,7 @@ REBASE_DUMP_OBJS = rebase-dump.$(O) rebase-db.$(O) $(LIBOBJS)
>  REBASE_DUMP_LIBS =
>  
>  PEFLAGS_OBJS = peflags.$(O) pechecksum.$(O) $(LIBOBJS)
> -PEFLAGS_LIBS =
> +PEFLAGS_LIBS = $(LIBIMAGEHELPER)
>  
>  SRC_DISTFILES = configure.ac configure Makefile.in \
>  	peflagsall.in rebaseall.in \
> @@ -109,8 +109,8 @@ rebase-dump$(EXEEXT): $(REBASE_DUMP_OBJS)
>  
>  rebase-dump.$(O):: rebase-dump.c rebase-db.h Makefile
>  
> -peflags$(EXEEXT): $(PEFLAGS_OBJS)
> -	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(PEFLAGS_OBJS)
> +peflags$(EXEEXT): $(PEFLAGS_OBJS) $(PEFLAGS_LIBS)
> +	$(CXX) $(CXXFLAGS) $(LDFLAGS) $(CXX_LDFLAGS) -o $@ $(PEFLAGS_OBJS) $(PEFLAGS_LIBS)
>  
>  peflags.$(O):: peflags.c pechecksum.h Makefile
>  
> diff --git a/imagehelper/Makefile.in b/imagehelper/Makefile.in
> index 0ed8e6c..a40cd8c 100755
> --- a/imagehelper/Makefile.in
> +++ b/imagehelper/Makefile.in
> @@ -75,11 +75,12 @@ LIB_TARGET=imagehelper
>  LIB_TARGET_FILE=libimagehelper.a
>  LIB_OBJS = objectfile.$(O) objectfilelist.$(O) sections.$(O) debug.$(O) \
>  	rebaseimage.$(O) checkimage.$(O) fiximage.$(O) getimageinfos.$(O) \
> -	bindimage.$(O)
> +	bindimage.$(O) pechecksum_update.$(O)
>  LIB_SRCS = objectfile.cc objectfilelist.cc sections.cc debug.cc \
>  	rebaseimage.cc checkimage.cc fiximage.cc getimageinfos.cc \
> -	bindimage.cc
> -LIB_HDRS = objectfilelist.h imagehelper.h sections.h objectfile.h
> +	bindimage.cc pechecksum_update.c
> +LIB_HDRS = objectfilelist.h imagehelper.h sections.h objectfile.h \
> +	pechecksum_update.h
>  
>  #
>  # (obsolete) applications
> diff --git a/imagehelper/imagehelper.h b/imagehelper/imagehelper.h
> index bafc106..8535311 100755
> --- a/imagehelper/imagehelper.h
> +++ b/imagehelper/imagehelper.h
> @@ -33,6 +33,10 @@ extern BOOL ReBaseChangeFileTime;
>  /* Set to TRUE, if rebasing should also drop the /DYNAMICBASE flag
>     from the PE flags. */
>  extern BOOL ReBaseDropDynamicbaseFlag;
> +/* Set to TRUE if ReBaseImage{64} should update the PE checksum.
> +   Cleared if the update failed due to inconsistent original
> +   checksum. */
> +extern BOOL ReBaseUpdateCheckSum;
>  
>  BOOL ReBaseImage64(
>    LPCSTR CurrentImageName,
> diff --git a/imagehelper/objectfile.cc b/imagehelper/objectfile.cc
> index 4dafa6a..651fcce 100755
> --- a/imagehelper/objectfile.cc
> +++ b/imagehelper/objectfile.cc
> @@ -24,6 +24,7 @@
>  #include <time.h>
>  
>  #include "objectfile.h"
> +#include "pechecksum_update.h"
>  
>  // read a dll into the cache
>  
> @@ -109,6 +110,15 @@ ObjectFile::ObjectFile(const char *aFileName, bool writeable)
>        FileName = strdup(name);
>      }
>  
> +  LARGE_INTEGER fs;
> +  if (!GetFileSizeEx(hfile, &fs))
> +    {
> +      CloseHandle(hfile);
> +      Error = 2;
> +      return;
> +    }
> +  FileSize = fs.QuadPart;
> +
>    hfilemapping = CreateFileMapping(hfile, NULL, writeable ? PAGE_READWRITE : PAGE_READONLY , 0, 0,  NULL);
>    if (hfilemapping == 0)
>      {
> @@ -139,8 +149,7 @@ ObjectFile::ObjectFile(const char *aFileName, bool writeable)
>        return;
>      }
>    // filesize big enough to allow at least reading the NT header?
> -  if (GetFileSize (hfile, NULL)
> -      < (char *) ntheader - (char *) dosheader + sizeof *ntheader)
> +  if (FileSize < (char *) ntheader - (char *) dosheader + sizeof *ntheader)
>      {
>        Error = 4;
>        return;
> @@ -178,9 +187,6 @@ ObjectFile::~ObjectFile()
>      CloseHandle(hfile);
>  }
>  
> -
> -
> -
>  int LinkedObjectFile::level = 0;
>  
>  LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) : ObjectFile(aFileName,writable)
> @@ -204,6 +210,12 @@ LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) : Objec
>    PIMAGE_NT_HEADERS32 ntheader32 = getNTHeader32 ();
>    PIMAGE_NT_HEADERS64 ntheader64 = getNTHeader64 ();
>  
> +  // Save original header for checksum update
> +  if (is64bit ())
> +    old_ntheader64 = *ntheader64;
> +  else
> +    old_ntheader32 = *ntheader32;
> +
>    Section *edata = sections->find(".edata");
>    if (edata)
>      exports = new Exports(*edata);
> @@ -231,9 +243,13 @@ LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) : Objec
>        imports = new Imports(*sections, dir);
>      }
>  
> +  uint32_t checksum;
> +  if (is64bit ())
> +    checksum = getNTHeader64 ()->OptionalHeader.CheckSum;
> +  else
> +    checksum = getNTHeader32 ()->OptionalHeader.CheckSum;
>  
> -
> -  relocs = new Relocations(*sections,".reloc");
> +  relocs = new Relocations(*sections, ".reloc", pe32_checksum_update_begin(checksum, FileSize));
>  
>  }
>  
> @@ -474,6 +490,38 @@ bool LinkedObjectFile::unbind(void)
>    return true;
>  }
>  
> +BOOL LinkedObjectFile::updateCheckSum()
> +{
> +  ULONG *p_checksum;
> +  if (is64bit ())
> +    p_checksum = &getNTHeader64 ()->OptionalHeader.CheckSum;
> +  else
> +    p_checksum = &getNTHeader32 ()->OptionalHeader.CheckSum;
> +
> +  // Add changes from NT header to checksum updated by relocs
> +  uint16_t checksum16;
> +  if (relocs)
> +    checksum16 = relocs->getCheckSum16();
> +  else
> +    checksum16 = pe32_checksum_update_begin(*p_checksum, FileSize);
> +
> +  if (is64bit ())
> +    checksum16 = pe32_checksum_update_add(checksum16, &old_ntheader64,
> +					  getNTHeader64 (),
> +					  sizeof(old_ntheader64));
> +  else
> +    checksum16 = pe32_checksum_update_add(checksum16, &old_ntheader32,
> +					 getNTHeader32 (),
> +					 sizeof(old_ntheader32));
> +
> +  uint32_t checksum = pe32_checksum_update_end(checksum16, FileSize);
> +  if (!checksum)
> +    return FALSE;
> +
> +  // Update the PE header
> +  *p_checksum = checksum;
> +  return TRUE;
> +}
>  
>  LinkedObjectFile::~LinkedObjectFile()
>  {
> diff --git a/imagehelper/objectfile.h b/imagehelper/objectfile.h
> index cd9509c..9deda1f 100755
> --- a/imagehelper/objectfile.h
> +++ b/imagehelper/objectfile.h
> @@ -92,6 +92,7 @@ class ObjectFile : public Base
>  
>    protected:
>      char *FileName;
> +    ULONG64 FileSize;
>      HANDLE hfile;
>      HANDLE hfilemapping;
>      LPVOID lpFileBase;
> @@ -140,12 +141,18 @@ class LinkedObjectFile : public ObjectFile
>        return exports;
>      }
>  
> +    BOOL updateCheckSum();
> +
>    protected:
>      Imports *imports;
>      Exports *exports;
>      Relocations *relocs;
>      static int level;
>      bool isPrinted;
> +    union {
> +      IMAGE_NT_HEADERS32 old_ntheader32;
> +      IMAGE_NT_HEADERS64 old_ntheader64;
> +    };
>    };
>  
>  #include "objectfilelist.h"
> diff --git a/imagehelper/pechecksum_update.c b/imagehelper/pechecksum_update.c
> new file mode 100644
> index 0000000..c15579d
> --- /dev/null
> +++ b/imagehelper/pechecksum_update.c
> @@ -0,0 +1,56 @@
> +/*
> + * PE32 checksum update
> + *
> + * Copyright (C) 2023 Christian Franke
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +#include "pechecksum_update.h"
> +
> +uint16_t pe32_checksum_update_begin(uint32_t checksum, unsigned long long filesize)
> +{
> +  if (!(0 < checksum && filesize < checksum && checksum - filesize <= 0xffffULL))
> +    return 0;
> +  return (uint16_t)(checksum - filesize);
> +}
> +
> +static inline uint16_t get_word(const void * p, unsigned i)
> +{
> +  const uint8_t * bp = p;
> +  return bp[i] | (bp[i + 1] << 8);
> +}
> +
> +uint16_t pe32_checksum_update_add(uint16_t checksum16, const void * old_block,
> +                                  const void * new_block, unsigned size)
> +{
> +  if (!(checksum16 && !((uintptr_t)old_block & 1)
> +        && !((uintptr_t)new_block & 1) && !(size & 1)))
> +    return 0;
> +  uint32_t sum = checksum16;
> +
> +  for (unsigned i = 0; i < size; i += 2) {
> +    uint16_t old_word = get_word(old_block, i);
> +    uint16_t new_word = get_word(new_block, i);
> +    if (new_word == old_word)
> +      continue;
> +    // Note: This does not work for updates of the first nonzero word of the file.
> +    // This does not matter because the first word "MZ" is never changed.
> +    if (old_word < sum) // Never return to 0, so no '<='.
> +      sum -= old_word;
> +    else // Revert previous 16-bit overflow, skip 0.
> +      sum += 0xffffU - old_word;
> +    sum += new_word;
> +    // If 16-bit overflow, skip 0.
> +    sum = (sum + (sum >> 16)) & 0xffffU;
> +  }
> +
> +  return (uint16_t)sum;
> +}
> +
> +uint32_t pe32_checksum_update_end(uint16_t checksum16, unsigned long long filesize)
> +{
> +  if (!(checksum16 && checksum16 + filesize < 0xffffffffULL))
> +    return 0;
> +  return (uint32_t)(checksum16 + filesize);
> +}
> diff --git a/imagehelper/pechecksum_update.h b/imagehelper/pechecksum_update.h
> new file mode 100644
> index 0000000..7a2c911
> --- /dev/null
> +++ b/imagehelper/pechecksum_update.h
> @@ -0,0 +1,52 @@
> +/*
> + * PE32 checksum update
> + *
> + * Copyright (C) 2023 Christian Franke
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +#ifndef PE_CHECKSUM_UPDATE_H
> +#define PE_CHECKSUM_UPDATE_H
> +
> +#include <stdint.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/*
> + * Begin checksum update sequence.
> + * checksum: Old 32-bit checksum from PE header.
> + * filesize: Size of PE file.
> + * return value: 16-bit checksum for p32_checksum_update_add()
> + * or 0 on error (the checksum itself is never 0).
> + */
> +uint16_t pe32_checksum_update_begin(uint32_t checksum, unsigned long long filesize);
> +
> +/*
> + * Update checksum.
> + * checksum16: Current 16-bit checksum from p32_checksum_update_begin()
> + * or this function.
> + * old_block: Address of old (original) data block, must be word aligned.
> + * new_block: Address of new (changed) data block, must be word aligned.
> + * size: Number of bytes of both data blocks, must be even.
> + * The PE checksum field may be included if identical in both blocks.
> + * return value: Updated 16-bit checksum or 0 on error.
> + */
> +uint16_t pe32_checksum_update_add(uint16_t checksum16, const void * old_block,
> +                                  const void * new_block, unsigned size);
> +
> +/*
> + * End checksum update sequence.
> + * checksum16: New 16-bit checksum from pe32_checksum_update_add().
> + * filesize: Size of PE file.
> + * return value: New 32-bit checksum for PE header or 0 on error.
> + */
> +uint32_t pe32_checksum_update_end(uint16_t checksum16, unsigned long long filesize);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* PE_CHECKSUM_UPDATE_H */
> diff --git a/imagehelper/rebaseimage.cc b/imagehelper/rebaseimage.cc
> index 2f1fb03..8827a14 100755
> --- a/imagehelper/rebaseimage.cc
> +++ b/imagehelper/rebaseimage.cc
> @@ -32,6 +32,7 @@
>  
>  BOOL ReBaseChangeFileTime = FALSE;
>  BOOL ReBaseDropDynamicbaseFlag = FALSE;
> +BOOL ReBaseUpdateCheckSum = FALSE;
>  
>  BOOL ReBaseImage64 (
>    LPCSTR CurrentImageName,
> @@ -141,6 +142,9 @@ BOOL ReBaseImage64 (
>  	  &= ~IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE;
>      }
>  
> +  if (ReBaseUpdateCheckSum && !dll.updateCheckSum())
> +    ReBaseUpdateCheckSum = FALSE;
> +
>    if (!fGoingDown)
>      *NewImageBase += *NewImageSize;
>  
> diff --git a/imagehelper/sections.cc b/imagehelper/sections.cc
> index dbf7da0..76f46ee 100755
> --- a/imagehelper/sections.cc
> +++ b/imagehelper/sections.cc
> @@ -22,8 +22,10 @@
>  
>  #include <iostream>
>  #include <iomanip>
> +#include <string.h>
>  
>  #include "sections.h"
> +#include "pechecksum_update.h"
>  
>  int Base::debug = 0;
>  
> @@ -270,10 +272,11 @@ void Imports::dump(const char *title)
>  //----- class Imports --------------------------------------------------
>  
>  
> -Relocations::Relocations(SectionList &sectionList, const char *sectionName)
> +Relocations::Relocations(SectionList &sectionList, const char *sectionName, uint16_t checksum16_)
>  {
>    sections = &sectionList;
>    Section *asection = sections->find(sectionName);
> +  checksum16 = checksum16_;
>    if (asection)
>      {
>        relocs = PIMAGE_BASE_RELOCATION (asection->getStartAddress());
> @@ -363,8 +366,11 @@ bool Relocations::fix(void)
>          std::cerr << "no errors found" << std::endl;
>      }
>    else
> -    if (debug)
> -      std::cerr << "corrupted relocation records fixed" << std::endl;
> +    {
> +      checksum16 = 0; // Checksum update not implemented.
> +      if (debug)
> +	std::cerr << "corrupted relocation records fixed" << std::endl;
> +    }
>    return true;
>  }
>  
> @@ -425,7 +431,12 @@ bool Relocations::relocate(int64_t difference)
>  		    << std::endl;
>  		  }
>  		int32_t *patch_adr = (int *)cursec->rva2real(location);
> +		unsigned cs_offs = (uintptr_t)patch_adr & 1; // 32-bit relocations may be not word aligned.
> +		const uint8_t *cs_adr = (const uint8_t *)patch_adr - cs_offs;
> +		unsigned cs_size = sizeof(uint32_t) + cs_offs * 2;
> +		uint8_t old_val[cs_size]; memcpy(old_val, cs_adr, cs_size);
>  		*patch_adr += difference;
> +		checksum16 = pe32_checksum_update_add(checksum16, old_val, cs_adr, cs_size);
>  	      }
>  	      break;
>  	    case IMAGE_REL_BASED_DIR64:
> @@ -440,7 +451,10 @@ bool Relocations::relocate(int64_t difference)
>  		    << std::endl;
>  		  }
>  		int64_t *patch_adr = (int64_t *)cursec->rva2real(location);
> +		int64_t old_val = *patch_adr;
>  		*patch_adr += difference;
> +		// Assume that 64-bit relocations are always word aligned.
> +		checksum16 = pe32_checksum_update_add(checksum16, &old_val, patch_adr, sizeof(old_val));
>  	      }
>  	      break;
>  	    default:
> diff --git a/imagehelper/sections.h b/imagehelper/sections.h
> index b317fa1..20dd24c 100755
> --- a/imagehelper/sections.h
> +++ b/imagehelper/sections.h
> @@ -199,7 +199,7 @@ class Relocations : SectionBase
>    {
>    public:
>      // create a relocation object using section named "section" from  the section list "sections"
> -    Relocations(SectionList &sectionList, const char *sectionName);
> +    Relocations(SectionList &sectionList, const char *sectionName, uint16_t checksum16_);
>  
>      // check for bad relocations
>      bool check(void);
> @@ -210,10 +210,16 @@ class Relocations : SectionBase
>      // precondition: fixed dll
>      bool relocate(int64_t difference);
>  
> +    uint16_t getCheckSum16() const
> +      {
> +	return checksum16;
> +      }
> +
>    private:
>      PIMAGE_BASE_RELOCATION relocs;
>      SectionList *sections;
>      int size;   // section size
> +    uint16_t checksum16;
>    };
>  
>  #endif
> diff --git a/peflags.c b/peflags.c
> index 1a61da7..917088b 100644
> --- a/peflags.c
> +++ b/peflags.c
> @@ -44,6 +44,7 @@
>  #include <windows.h>
>  
>  #include "pechecksum.h"
> +#include "imagehelper/pechecksum_update.h"
>  
>  #if defined(__MSYS__)
>  /* MSYS has no strtoull */
> @@ -67,12 +68,12 @@ static WORD coff_characteristics_show;
>  static WORD pe_characteristics_set;
>  static WORD pe_characteristics_clr;
>  static WORD pe_characteristics_show;
> -static short checksum_show;
> -static short checksum_update; /* 1 = update after changes, 2 = fix always */
> +static short checksum_option; /* 1 = show, 2 = verify, 3 = fix */
>  
>  typedef struct
>  {
>    const char *pathname;
> +  unsigned long long filesize;
>    PIMAGE_DOS_HEADER dosheader;
>    union
>      {
> @@ -507,24 +508,48 @@ do_mark (const char *pathname)
>  
>      }
>  
> -  /* Any actual changes? */
> -  if (pep->is_64bit)
> -    {
> -      changed = memcmp(&old_ntheader.ntheader64, pep->ntheader64,
> -		       sizeof(old_ntheader.ntheader64));
> -      hdr_checksum = pep->ntheader64->OptionalHeader.CheckSum;
> -    }
> -  else
> -    {
> -      changed = memcmp(&old_ntheader.ntheader32, pep->ntheader64,
> -		       sizeof(old_ntheader.ntheader32));
> -      hdr_checksum = pep->ntheader32->OptionalHeader.CheckSum;
> -    }
> +  /* Update the checksum. */
> +  changed = FALSE;
> +  {
> +    const void *p_oldheader, *p_newheader;
> +    unsigned headersize;
> +    ULONG *p_checksum;
> +    if (pep->is_64bit)
> +      {
> +	p_oldheader = &old_ntheader.ntheader64;
> +	p_newheader = pep->ntheader64;
> +	headersize = sizeof(old_ntheader.ntheader64);
> +	p_checksum = &pep->ntheader64->OptionalHeader.CheckSum;
> +      }
> +    else
> +      {
> +	p_oldheader = &old_ntheader.ntheader32;
> +	p_newheader = pep->ntheader32;
> +	headersize = sizeof(old_ntheader.ntheader32);
> +	p_checksum = &pep->ntheader32->OptionalHeader.CheckSum;
> +      }
> +
> +    hdr_checksum = pe32_checksum_update_end(
> +      pe32_checksum_update_add(
> +	pe32_checksum_update_begin(*p_checksum, pep->filesize),
> +	p_oldheader, p_newheader, headersize
> +      ),
> +      pep->filesize
> +    );
> +
> +    if (!hdr_checksum)
> +      fprintf(stderr, "%s: Checksum update failed\n", pathname);
> +    else if (*p_checksum != hdr_checksum)
> +      {
> +	changed = TRUE;
> +	*p_checksum = hdr_checksum;
> +      }
> +  }
>  
>    pe_close (pep);
>  
> -  /* Checksum calculation requires re-open as a regular file. */
> -  if (checksum_show || checksum_update)
> +  /* Checksum re-calculation requires re-open as a regular file. */
> +  if (checksum_option)
>      do_checksum (pathname, indent, changed, hdr_checksum);
>    return 0;
>  }
> @@ -625,16 +650,16 @@ static void
>  do_checksum (const char *pathname, int indent, int changed, ULONG hdr_checksum)
>  {
>    const char name[] = "PE file checksum";
> -  if (checksum_show || (checksum_update && changed) || checksum_update > 1)
> +  if (checksum_option > 1)
>      {
>        int fd;
>        unsigned new_checksum, old_checksum;
> -      if ((fd = open (pathname, (checksum_update ? O_RDWR : O_RDONLY) | O_BINARY)) == -1)
> +      if ((fd = open (pathname, (checksum_option > 2 ? O_RDWR : O_RDONLY) | O_BINARY)) == -1)
>  	{
>  	  fprintf (stderr, "%s: Unable to reopen, errno=%d\n", pathname, errno);
>  	  return;
>  	}
> -      new_checksum = pe32_checksum (fd, checksum_update, &old_checksum);
> +      new_checksum = pe32_checksum (fd, checksum_option > 2, &old_checksum);
>        close(fd);
>        if (!new_checksum)
>  	{
> @@ -650,19 +675,19 @@ do_checksum (const char *pathname, int indent, int changed, ULONG hdr_checksum)
>  	}
>  
>        if (new_checksum == old_checksum)
> -	  printf ("%*s%-24s: 0x%08x (%sverified)\n", indent, "", name,
> -		  old_checksum, (checksum_update ? "unchanged, " : ""));
> -      else if (checksum_update)
> +	  printf ("%*s%-24s: 0x%08x (%schanged, verified)\n", indent, "", name,
> +	          old_checksum, (changed ? "" : "un"));
> +      else if (checksum_option > 2)
>  	  printf ("%*s%-24s: 0x%08x (updated from 0x%08x)\n", indent, "", name,
>  		  new_checksum, old_checksum);
>        else
>  	  printf ("%*s%-24s: 0x%08x (*needs update to 0x%08x*)\n", indent, "", name,
>  		  old_checksum, new_checksum);
>      }
> -  else /* (!checksum_show && checksum_update == 1 && !changed) */
> +  else /* (checksum_option == 1) */
>      {
> -      printf ("%*s%-24s: 0x%08x (unchanged, not verified)\n", indent, "", name,
> -	      hdr_checksum);
> +      printf ("%*s%-24s: 0x%08x (%schanged, not verified)\n", indent, "", name,
> +	      hdr_checksum, (changed ? "" : "un"));
>      }
>  }
>  
> @@ -767,11 +792,7 @@ handle_checksum_option (const char *option_name,
>  {
>    int bool_value;
>    if (!option_arg)
> -    {
> -      checksum_show = 1;
> -      if (handle_any_sizeof == DONT_HANDLE)
> -	handle_any_sizeof = DO_READ;
> -    }
> +    checksum_option = 1;
>    else
>      {
>        if (string_to_bool (option_arg, &bool_value) != 0)
> @@ -781,9 +802,12 @@ handle_checksum_option (const char *option_name,
>            short_usage (stderr);
>            exit (1);
>          }
> -      checksum_update = (bool_value ? 2 : 1);
> -      handle_any_sizeof = DO_WRITE;
> +      checksum_option = (!bool_value ? 2 : 3);
>      }
> +  if (checksum_option > 2)
> +    handle_any_sizeof = DO_WRITE;
> +  else if (handle_any_sizeof == DONT_HANDLE)
> +    handle_any_sizeof = DO_READ;
>  }
>  
>  void
> @@ -1042,6 +1066,11 @@ pe_open (const char *path, BOOL writing)
>    int fd;
>    void *map;
>    
> +  /* Get filesize for checksum update. */
> +  struct stat st;
> +  if (stat (path, &st))
> +    return NULL;
> +
>    fd = open (path, O_BINARY | (writing ? O_RDWR : O_RDONLY));
>    if (fd == -1)
>      return NULL;
> @@ -1053,6 +1082,7 @@ pe_open (const char *path, BOOL writing)
>        return NULL;
>      }
>    pef.pathname = path;
> +  pef.filesize = st.st_size;
>    pef.dosheader = (PIMAGE_DOS_HEADER) map;
>    pef.ntheader32 = (PIMAGE_NT_HEADERS32)
>  		   ((PBYTE) pef.dosheader + pef.dosheader->e_lfanew);
> @@ -1233,9 +1263,12 @@ help (FILE *f)
>  "                              than 2 GB.\n"
>  "  -S, --sepdbg       [BOOL]   Debugging information was removed and stored\n"
>  "                              separately in another file.\n"
> -"  -k, --checksum     [BOOL]   Displays (no argument), updates after changes\n"
> -"                              only (false) or always fixes (true) the file\n"
> -"                              checksum in the PE header.\n"
> +"  -k, --checksum     [BOOL]   Displays (no argument), verifies (false) or\n"
> +"                              always fixes (true) the file checksum in the PE\n"
> +"                              header.  By default, the checksum is updated\n"
> +"                              without reading the whole file under the\n"
> +"                              assumption that the original checksum was\n"
> +"                              correct.\n"
>  "  -x, --stack-reserve [NUM]   Reserved stack size of the process in bytes.\n"
>  "  -X, --stack-commit  [NUM]   Initial commited portion of the process stack\n"
>  "                              in bytes.\n"
> @@ -1272,7 +1305,7 @@ help (FILE *f)
>  "For flag values, to set a value, and display the results symbolic, repeat the\n"
>  "option:  --tsaware=true --tsaware -d0 -d\n"
>  "The time of last modification of each file is preserved unless the checksum\n"
> -"has been updated.\n"
> +"has been fixed by option --checksum=true\n"
>  "\n", f);
>  }
>  
> diff --git a/rebase.c b/rebase.c
> index 50cc79f..20a9902 100644
> --- a/rebase.c
> +++ b/rebase.c
> @@ -1190,7 +1190,7 @@ print_overlapped ()
>      }
>  }
>  
> -static BOOL
> +static int
>  update_checksum (const char *pathname)
>  {
>    int fd, err;
> @@ -1200,7 +1200,7 @@ update_checksum (const char *pathname)
>      {
>        fprintf (stderr, "%s: failed to reopen for checksum update: %s\n",
>  	       pathname, strerror (errno));
> -      return FALSE;
> +      return -1;
>      }
>    new_checksum = pe32_checksum (fd, 1, &old_checksum);
>    err = errno;
> @@ -1210,10 +1210,10 @@ update_checksum (const char *pathname)
>        fprintf (stderr, "%s: checksum update failed: %s\n", pathname,
>  	       strerror (err));
>        /* Assume file is unchanged. */
> -      return FALSE;
> +      return -1;
>      }
>  
> -  return (new_checksum != old_checksum);
> +  return (new_checksum != old_checksum ? 1 : 0);
>  }
>  
>  BOOL
> @@ -1222,6 +1222,7 @@ rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag)
>    ULONG64 old_image_base, prev_new_image_base;
>    ULONG old_image_size, new_image_size;
>    BOOL status;
> +  const char *checksum_msg;
>  
>    /* Skip if not writable. */
>    if (access (pathname, W_OK) == -1)
> @@ -1231,6 +1232,9 @@ rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag)
>        return TRUE;
>      }
>  
> +  /* Try to update the checksum. */
> +  ReBaseUpdateCheckSum = TRUE;
> +
>    /* Calculate next base address, if rebasing down. */
>    if (down_flag)
>      *new_image_base -= offset;
> @@ -1307,8 +1311,17 @@ retry:
>      }
>  #endif
>  
> -  /* Update checksum, if requested. */
> -  status = (checksum_flag ? update_checksum (pathname) : FALSE);
> +
> +  /* Fix checksum, if requested or incremental update failed. */
> +  if (checksum_flag || !ReBaseUpdateCheckSum)
> +    switch (update_checksum (pathname))
> +      {
> +	case 0:  checksum_msg = ", checksum verified"; break;
> +	case 1:  checksum_msg = ", checksum fixed"; break;
> +	default: checksum_msg = ", checksum update failed"; break;
> +      }
> +  else
> +    checksum_msg = "";
>  
>    /* Display rebase results, if verbose. */
>    if (verbose)
> @@ -1316,9 +1329,7 @@ retry:
>        printf ("%s: new base = %" PRIx64 ", new size = %x%s\n",
>  	      pathname,
>  	      (uint64_t) ((down_flag) ? *new_image_base : prev_new_image_base),
> -	      (uint32_t) (new_image_size + offset),
> -	      (checksum_flag ? (status ? ", checksum updated" :
> -	      ", checksum unchanged") : ""));
> +	      (uint32_t) (new_image_size + offset), checksum_msg);
>      }
>  
>    /* Calculate next base address, if rebasing up. */
> @@ -1662,10 +1673,12 @@ Rebase PE files, usually DLLs, to a specified address or address range.\n\
>    One of the options -b, -s or -i is mandatory.  If no rebase database exists\n\
>    yet, -b is required together with -s.\n\
>  \n\
> -  -c, --checksum          Update the file's checksum in the PE header if the\n\
> -                          file has been successfully rebased.  This also bumps\n\
> -                          the file's modification time (like -t) if the\n\
> -                          checksum has been changed.\n\
> +  -c, --checksum          Fix the file's checksum in the PE header if the file\n\
> +                          has been successfully rebased.  This also bumps the\n\
> +                          file's modification time (like -t) if the checksum\n\
> +                          has been changed.  By default, the checksum is\n\
> +                          updated without reading the whole file under the\n\
> +                          assumption that the original checksum was correct.\n\
>    -d, --down              Treat the BaseAddress as upper ceiling and rebase\n\
>                            files top-down from there.  Without this option the\n\
>                            files are rebased from BaseAddress bottom-up.\n\
> -- 
> 2.39.0
> 

Re: [PATCH rebase 1/2] Always update the file checksum in the PE header

[Christian Franke]

Corinna Vinschen wrote:
> On Aug 12 14:26, Christian Franke via Cygwin-apps wrote:
>> This patch is still experimental, but tested with all /bin/cyg*.dll from my
>> installation.
> Does that mean I shouldn't apply it for now, or do you want me to
> apply it but not create a new release yet?
>

Meantime I did more tests, also a few with 32-bit DLLs, which all 
succeeded. So it should be safe to use.

In conjunction with the "Don't update the PE header timestamp unless -t 
is used" patch, rebase results are reproducible.

-- 
Regards,
Christian

Re: [PATCH rebase 1/2] Always update the file checksum in the PE header

[Corinna Vinschen]

On Aug 15 11:29, Christian Franke via Cygwin-apps wrote:
> Corinna Vinschen wrote:
> > On Aug 12 14:26, Christian Franke via Cygwin-apps wrote:
> > > This patch is still experimental, but tested with all /bin/cyg*.dll from my
> > > installation.
> > Does that mean I shouldn't apply it for now, or do you want me to
> > apply it but not create a new release yet?
> > 
> 
> Meantime I did more tests, also a few with 32-bit DLLs, which all succeeded.
> So it should be safe to use.
> 
> In conjunction with the "Don't update the PE header timestamp unless -t is
> used" patch, rebase results are reproducible.

Pushed.  I've started deploying a rebase 4.6.6.


Thanks,
Corinna