binutils: Add security patches

Before this commit, the `.CRT` section (which contains pointers to image TLS callbacks) and the `.idata` section (which contains `__imp_` pointers to dllimport functions) that had been created by the GNU linker were writeable. As these function pointers are essential for programs to be functional, they could be exploited to inject malicious code, like the well-known IFUNC backdoor in XZ Utils. The Microsoft linker does not create these sections as writeable; instead, they seem to be merged into `.rdata` and are not modifiable, unless unprotected. LLD also does the same, suggesting these sections be merged into `.rdata`: https://github.com/llvm/llvm-project/blob/ebeb56af5f8f1ff9da8f5a7e98348f460d223de1/lld/COFF/Driver.cpp#L2034-L2048 This commit includes the following countermeasures: 1. Patch 3005 ensures the final `.CRT` and `.idata` sections (if any) will not be writeable in the final image. 2. Patch 3006 merges known sub-sections of `.CRT`, as well as `.ctors` and `.dtors` (which were merged into `.text` instead), into `.rdata`. Merging `.idata` into `.rdata` seems to prevent dllimport from working, so there's still an `.idata` section in the final image. See also: https://stackoverflow.com/questions/22651433/pe-idata-section Reference: https://sourceware.org/bugzilla/show_bug.cgi?id=32264 Signed-off-by: LIU Hao <[email protected]>
msys2 · Oct 23, 2024 · 9531b24 · 9531b24
1 parent f5765c1
commit 9531b24
Show file tree

Hide file tree

Showing 3 changed files with 345 additions and 3 deletions.
diff --git a/mingw-w64-binutils/3005-Make-.CRT-and-.idata-read-only.patch b/mingw-w64-binutils/3005-Make-.CRT-and-.idata-read-only.patch
@@ -0,0 +1,31 @@
+From 4af2c5ca7d2df81c4ae4a1e63aab92c323a75a43 Mon Sep 17 00:00:00 2001
+From: LIU Hao <[email protected]>
+Date: Tue, 22 Oct 2024 15:06:17 +0800
+Subject: [PATCH] Make `.CRT` and `.idata` read-only
+
+Signed-off-by: LIU Hao <[email protected]>
+---
+ bfd/peXXigen.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
+index 57cb3f5a184..2f503d264a6 100644
+--- a/bfd/peXXigen.c
++++ b/bfd/peXXigen.c
+@@ -995,11 +995,12 @@ _bfd_XXi_swap_scnhdr_out (bfd * abfd, void * in, void * out)
+
+     pe_required_section_flags known_sections [] =
+       {
++	{ ".CRT",   IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA },
+ 	{ ".arch",  IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_DISCARDABLE | IMAGE_SCN_ALIGN_8BYTES },
+ 	{ ".bss",   IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_UNINITIALIZED_DATA | IMAGE_SCN_MEM_WRITE },
+ 	{ ".data",  IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_WRITE },
+ 	{ ".edata", IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA },
+-	{ ".idata", IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_WRITE },
++	{ ".idata", IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA },
+ 	{ ".pdata", IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA },
+ 	{ ".rdata", IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA },
+ 	{ ".reloc", IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_DISCARDABLE },
+-- 
+2.47.0
+
diff --git a/mingw-w64-binutils/3006-Merge-.CRT-.ctors-and-.dtors-into-.rdata.patch b/mingw-w64-binutils/3006-Merge-.CRT-.ctors-and-.dtors-into-.rdata.patch
@@ -0,0 +1,303 @@
+From b160435f328feb3638f55e88c31aa4c4ab3634be Mon Sep 17 00:00:00 2001
+From: LIU Hao <[email protected]>
+Date: Tue, 22 Oct 2024 22:28:07 +0800
+Subject: [PATCH] Merge `.CRT`, `.ctors` and `.dtors` into `.rdata`
+
+Signed-off-by: LIU Hao <[email protected]>
+---
+ ld/scripttempl/pe.sc  | 103 ++++++++++++++++++++++--------------------
+ ld/scripttempl/pep.sc | 102 ++++++++++++++++++++++-------------------
+ 2 files changed, 110 insertions(+), 95 deletions(-)
+
+diff --git a/ld/scripttempl/pe.sc b/ld/scripttempl/pe.sc
+index 70f5194b02f..30b0f5b653d 100644
+--- a/ld/scripttempl/pe.sc
++++ b/ld/scripttempl/pe.sc
+@@ -44,6 +44,7 @@ if test "${RELOCATING}"; then
+   R_CRT_XL='KEEP (*(SORT(.CRT$XL*)))  /* TLS callbacks */'
+   R_CRT_XP='KEEP (*(SORT(.CRT$XP*)))  /* Pre-termination */'
+   R_CRT_XT='KEEP (*(SORT(.CRT$XT*)))  /* Termination */'
++  R_CRT_XD='KEEP (*(SORT(.CRT$XD*)))  /* Dynamic TLS Initializer */'
+   R_TLS='
+     KEEP (*(.tls$AAA))
+     KEEP (*(.tls))
+@@ -65,6 +66,7 @@ else
+   R_CRT_XL=
+   R_CRT_XP=
+   R_CRT_XT=
++  R_CRT_XD=
+   R_TLS='*(.tls)'
+   R_RSRC='*(.rsrc)'
+ fi
+@@ -97,6 +99,40 @@ SECTIONS
+     ${RELOCATING+ *(.gnu.linkonce.t.*)}
+     ${RELOCATING+*(.glue_7t)}
+     ${RELOCATING+*(.glue_7)}
++    ${RELOCATING+KEEP (*(SORT_NONE(.fini)))}
++    ${RELOCATING+/* ??? Why is .gcc_exc here?  */}
++    ${RELOCATING+ *(.gcc_exc)}
++    ${RELOCATING+PROVIDE (etext = .);}
++    ${RELOCATING+PROVIDE (_etext = .);}
++    ${RELOCATING+ KEEP (*(.gcc_except_table))}
++  }
++
++  /* The Cygwin32 library uses a section to avoid copying certain data
++     on fork.  This used to be named ".data$nocopy".  The linker used
++     to include this between __data_start__ and __data_end__, but that
++     breaks building the cygwin32 dll.  Instead, we name the section
++     ".data_cygwin_nocopy" and explicitly include it after __data_end__. */
++
++  .data ${RELOCATING+BLOCK(__section_alignment__)} :
++  {
++    ${RELOCATING+__data_start__ = . ;}
++    *(.data)
++    ${RELOCATING+*(.data2)}
++    ${R_DATA}
++    KEEP(*(.jcr))
++    ${RELOCATING+__data_end__ = . ;}
++    ${RELOCATING+*(.data_cygwin_nocopy)}
++  }
++
++  .rdata ${RELOCATING+BLOCK(__section_alignment__)} :
++  {
++    ${R_RDATA}
++    . = ALIGN(4);
++    ${RELOCATING+__rt_psrelocs_start = .;}
++    ${RELOCATING+KEEP(*(.rdata_runtime_pseudo_reloc))}
++    ${RELOCATING+__rt_psrelocs_end = .;}
++
++    /* .ctors & .dtors */
+     ${CONSTRUCTING+
+        /* Note: we always define __CTOR_LIST__ and ___CTOR_LIST__ here,
+           we do not PROVIDE them.  This is because the ctors.o startup
+@@ -136,39 +172,28 @@ SECTIONS
+        KEEP(*(SORT_BY_NAME(.dtors.*)));
+        LONG (0);
+      }
+-    ${RELOCATING+KEEP (*(SORT_NONE(.fini)))}
+-    ${RELOCATING+/* ??? Why is .gcc_exc here?  */}
+-    ${RELOCATING+ *(.gcc_exc)}
+-    ${RELOCATING+PROVIDE (etext = .);}
+-    ${RELOCATING+PROVIDE (_etext = .);}
+-    ${RELOCATING+ KEEP (*(.gcc_except_table))}
+-  }
+-
+-  /* The Cygwin32 library uses a section to avoid copying certain data
+-     on fork.  This used to be named ".data$nocopy".  The linker used
+-     to include this between __data_start__ and __data_end__, but that
+-     breaks building the cygwin32 dll.  Instead, we name the section
+-     ".data_cygwin_nocopy" and explicitly include it after __data_end__. */
+
+-  .data ${RELOCATING+BLOCK(__section_alignment__)} :
+-  {
+-    ${RELOCATING+__data_start__ = . ;}
+-    *(.data)
+-    ${RELOCATING+*(.data2)}
+-    ${R_DATA}
+-    KEEP(*(.jcr))
+-    ${RELOCATING+__data_end__ = . ;}
+-    ${RELOCATING+*(.data_cygwin_nocopy)}
++    /* .CRT */
++    ${RELOCATING+___crt_xc_start__ = . ;}
++    ${R_CRT_XC}
++    ${RELOCATING+___crt_xc_end__ = . ;}
++    ${RELOCATING+___crt_xi_start__ = . ;}
++    ${R_CRT_XI}
++    ${RELOCATING+___crt_xi_end__ = . ;}
++    ${RELOCATING+___crt_xl_start__ = . ;}
++    ${R_CRT_XL}
++    /* ___crt_xl_end__ is defined in the TLS Directory support code */
++    ${RELOCATING+___crt_xp_start__ = . ;}
++    ${R_CRT_XP}
++    ${RELOCATING+___crt_xp_end__ = . ;}
++    ${RELOCATING+___crt_xt_start__ = . ;}
++    ${R_CRT_XT}
++    ${RELOCATING+___crt_xt_end__ = . ;}
++    ${RELOCATING+___crt_xd_start__ = . ;}
++    ${R_CRT_XD}
++    ${RELOCATING+___crt_xd_end__ = . ;}
+   }
+
+-  .rdata ${RELOCATING+BLOCK(__section_alignment__)} :
+-  {
+-    ${R_RDATA}
+-    . = ALIGN(4);
+-    ${RELOCATING+__rt_psrelocs_start = .;}
+-    ${RELOCATING+KEEP(*(.rdata_runtime_pseudo_reloc))}
+-    ${RELOCATING+__rt_psrelocs_end = .;}
+-  }
+   ${RELOCATING+__rt_psrelocs_size = __rt_psrelocs_end - __rt_psrelocs_start;}
+   ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+   ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+@@ -218,24 +243,6 @@ SECTIONS
+     ${RELOCATING+__IAT_end__ = .;}
+     ${R_IDATA67}
+   }
+-  .CRT ${RELOCATING+BLOCK(__section_alignment__)} :
+-  {
+-    ${RELOCATING+___crt_xc_start__ = . ;}
+-    ${R_CRT_XC}
+-    ${RELOCATING+___crt_xc_end__ = . ;}
+-    ${RELOCATING+___crt_xi_start__ = . ;}
+-    ${R_CRT_XI}
+-    ${RELOCATING+___crt_xi_end__ = . ;}
+-    ${RELOCATING+___crt_xl_start__ = . ;}
+-    ${R_CRT_XL}
+-    /* ___crt_xl_end__ is defined in the TLS Directory support code */
+-    ${RELOCATING+___crt_xp_start__ = . ;}
+-    ${R_CRT_XP}
+-    ${RELOCATING+___crt_xp_end__ = . ;}
+-    ${RELOCATING+___crt_xt_start__ = . ;}
+-    ${R_CRT_XT}
+-    ${RELOCATING+___crt_xt_end__ = . ;}
+-  }
+
+   /* Windows TLS expects .tls\$AAA to be at the start and .tls\$ZZZ to be
+      at the end of section.  This is important because _tls_start MUST
+diff --git a/ld/scripttempl/pep.sc b/ld/scripttempl/pep.sc
+index 63039f11574..afb8272d515 100644
+--- a/ld/scripttempl/pep.sc
++++ b/ld/scripttempl/pep.sc
+@@ -45,6 +45,7 @@ if test "${RELOCATING}"; then
+   R_CRT_XL='KEEP (*(SORT(.CRT$XL*)))  /* TLS callbacks */'
+   R_CRT_XP='KEEP (*(SORT(.CRT$XP*)))  /* Pre-termination */'
+   R_CRT_XT='KEEP (*(SORT(.CRT$XT*)))  /* Termination */'
++  R_CRT_XD='KEEP (*(SORT(.CRT$XD*)))  /* Dynamic TLS Initializer */'
+   R_TLS='
+     KEEP (*(.tls$AAA))
+     KEEP (*(.tls))
+@@ -66,6 +67,7 @@ else
+   R_CRT_XL=
+   R_CRT_XP=
+   R_CRT_XT=
++  R_CRT_XD=
+   R_TLS='*(.tls)'
+   R_RSRC='*(.rsrc)'
+ fi
+@@ -99,6 +101,40 @@ SECTIONS
+     ${RELOCATING+*(.glue_7t)}
+     ${RELOCATING+*(.glue_7)}
+     ${CONSTRUCTING+. = ALIGN(8);}
++    ${RELOCATING+KEEP (*(SORT_NONE(.fini)))}
++    ${RELOCATING+/* ??? Why is .gcc_exc here?  */}
++    ${RELOCATING+ *(.gcc_exc)}
++    ${RELOCATING+PROVIDE (etext = .);}
++    ${RELOCATING+ KEEP (*(.gcc_except_table))}
++  }
++
++  /* The Cygwin32 library uses a section to avoid copying certain data
++     on fork.  This used to be named ".data$nocopy".  The linker used
++     to include this between __data_start__ and __data_end__, but that
++     breaks building the cygwin32 dll.  Instead, we name the section
++     ".data_cygwin_nocopy" and explicitly include it after __data_end__. */
++
++  .data ${RELOCATING+BLOCK(__section_alignment__)} :
++  {
++    ${RELOCATING+__data_start__ = . ;}
++    *(.data)
++    ${RELOCATING+*(.data2)}
++    ${R_DATA}
++    KEEP(*(.jcr))
++    ${RELOCATING+__data_end__ = . ;}
++    ${RELOCATING+*(.data_cygwin_nocopy)}
++  }
++
++  .rdata ${RELOCATING+BLOCK(__section_alignment__)} :
++  {
++    ${R_RDATA}
++    . = ALIGN(4);
++    ${RELOCATING+__rt_psrelocs_start = .;}
++    ${RELOCATING+KEEP(*(.rdata_runtime_pseudo_reloc))}
++    ${RELOCATING+__rt_psrelocs_end = .;}
++
++    /* .ctors & .dtors */
++    ${CONSTRUCTING+. = ALIGN(8);}
+     ${CONSTRUCTING+
+        /* Note: we always define __CTOR_LIST__ and ___CTOR_LIST__ here,
+           we do not PROVIDE them.  This is because the ctors.o startup
+@@ -138,38 +174,28 @@ SECTIONS
+        KEEP (*(SORT_BY_NAME(.dtors.*)));
+        LONG (0); LONG (0);
+      }
+-    ${RELOCATING+KEEP (*(SORT_NONE(.fini)))}
+-    ${RELOCATING+/* ??? Why is .gcc_exc here?  */}
+-    ${RELOCATING+ *(.gcc_exc)}
+-    ${RELOCATING+PROVIDE (etext = .);}
+-    ${RELOCATING+ KEEP (*(.gcc_except_table))}
+-  }
+
+-  /* The Cygwin32 library uses a section to avoid copying certain data
+-     on fork.  This used to be named ".data$nocopy".  The linker used
+-     to include this between __data_start__ and __data_end__, but that
+-     breaks building the cygwin32 dll.  Instead, we name the section
+-     ".data_cygwin_nocopy" and explicitly include it after __data_end__. */
+-
+-  .data ${RELOCATING+BLOCK(__section_alignment__)} :
+-  {
+-    ${RELOCATING+__data_start__ = . ;}
+-    *(.data)
+-    ${RELOCATING+*(.data2)}
+-    ${R_DATA}
+-    KEEP(*(.jcr))
+-    ${RELOCATING+__data_end__ = . ;}
+-    ${RELOCATING+*(.data_cygwin_nocopy)}
++    /* .CRT */
++    ${RELOCATING+___crt_xc_start__ = . ;}
++    ${R_CRT_XC}
++    ${RELOCATING+___crt_xc_end__ = . ;}
++    ${RELOCATING+___crt_xi_start__ = . ;}
++    ${R_CRT_XI}
++    ${RELOCATING+___crt_xi_end__ = . ;}
++    ${RELOCATING+___crt_xl_start__ = . ;}
++    ${R_CRT_XL}
++    /* ___crt_xl_end__ is defined in the TLS Directory support code */
++    ${RELOCATING+___crt_xp_start__ = . ;}
++    ${R_CRT_XP}
++    ${RELOCATING+___crt_xp_end__ = . ;}
++    ${RELOCATING+___crt_xt_start__ = . ;}
++    ${R_CRT_XT}
++    ${RELOCATING+___crt_xt_end__ = . ;}
++    ${RELOCATING+___crt_xd_start__ = . ;}
++    ${R_CRT_XD}
++    ${RELOCATING+___crt_xd_end__ = . ;}
+   }
+
+-  .rdata ${RELOCATING+BLOCK(__section_alignment__)} :
+-  {
+-    ${R_RDATA}
+-    . = ALIGN(4);
+-    ${RELOCATING+__rt_psrelocs_start = .;}
+-    ${RELOCATING+KEEP(*(.rdata_runtime_pseudo_reloc))}
+-    ${RELOCATING+__rt_psrelocs_end = .;}
+-  }
+   ${RELOCATING+__rt_psrelocs_size = __rt_psrelocs_end - __rt_psrelocs_start;}
+   ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+   ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+@@ -224,24 +250,6 @@ SECTIONS
+     ${RELOCATING+__IAT_end__ = .;}
+     ${R_IDATA67}
+   }
+-  .CRT ${RELOCATING+BLOCK(__section_alignment__)} :
+-  {
+-    ${RELOCATING+___crt_xc_start__ = . ;}
+-    ${R_CRT_XC}
+-    ${RELOCATING+___crt_xc_end__ = . ;}
+-    ${RELOCATING+___crt_xi_start__ = . ;}
+-    ${R_CRT_XI}
+-    ${RELOCATING+___crt_xi_end__ = . ;}
+-    ${RELOCATING+___crt_xl_start__ = . ;}
+-    ${R_CRT_XL}
+-    /* ___crt_xl_end__ is defined in the TLS Directory support code */
+-    ${RELOCATING+___crt_xp_start__ = . ;}
+-    ${R_CRT_XP}
+-    ${RELOCATING+___crt_xp_end__ = . ;}
+-    ${RELOCATING+___crt_xt_start__ = . ;}
+-    ${R_CRT_XT}
+-    ${RELOCATING+___crt_xt_end__ = . ;}
+-  }
+
+   /* Windows TLS expects .tls\$AAA to be at the start and .tls\$ZZZ to be
+      at the end of the .tls section.  This is important because _tls_start MUST
+-- 
+2.47.0
+
diff --git a/mingw-w64-binutils/PKGBUILD b/mingw-w64-binutils/PKGBUILD
@@ -6,7 +6,7 @@ _realname=binutils
 pkgbase=mingw-w64-${_realname}
 pkgname=("${MINGW_PACKAGE_PREFIX}-${_realname}")
 pkgver=2.43.1
-pkgrel=1
+pkgrel=2
 pkgdesc="A set of programs to assemble and manipulate binary and object files (mingw-w64)"
 arch=('any')
 mingw_arch=('mingw32' 'mingw64' 'ucrt64')
@@ -32,7 +32,8 @@ source=(https://ftp.gnu.org/gnu/binutils/${_realname}-${pkgver}.tar.bz2{,.sig}
         reproducible-import-libraries.patch
         libiberty-unlink-handle-windows-nul.patch
         3001-hack-libiberty-link-order.patch
-)
+        3005-Make-.CRT-and-.idata-read-only.patch
+        3006-Merge-.CRT-.ctors-and-.dtors-into-.rdata.patch)
 sha256sums=('becaac5d295e037587b63a42fad57fe3d9d7b83f478eb24b67f9eec5d0f1872f'
             'SKIP'
             '2c99345fc575c3a060d6677537f636c6c4154fac0fde508070f3b6296c1060d4'
@@ -42,7 +43,9 @@ sha256sums=('becaac5d295e037587b63a42fad57fe3d9d7b83f478eb24b67f9eec5d0f1872f'
             'd584f1cd9e94cba0e9b27625c4acc8ad5242cd625c9b44839d42fc116072568c'
             'a094660ec95996c00b598429843b7869037732146442af567ada9f539bd40480'
             '7ccbd418695733c50966068fa9755a6abb156f53af23701d2bc097c63e9e0030'
-            '604628156c08f3e361de60329af250fab6839e23e61e289f8369a7e18a04e277')
+            '604628156c08f3e361de60329af250fab6839e23e61e289f8369a7e18a04e277'
+            'efb74f5ce78eaa0fcaca6d7418561ffdf7921784950446cd7e36579609b78488'
+            '300e273d3523ab646932687897945746d9fe961e6a31bcc0a62a015b697487e2')
 validpgpkeys=('EAF1C276A747E9ED86210CBAC3126D3B4AE55E93'  # Tristan Gingold <[email protected]>
               '3A24BC1E8FB409FA9F14371813FCEF89DD9E3C4F') # Nick Clifton <[email protected]>
 
@@ -62,6 +65,11 @@ prepare() {
     0010-bfd-Increase-_bfd_coff_max_nscns-to-65279.patch \
     0110-binutils-mingw-gnu-print.patch
 
+  # https://sourceware.org/bugzilla/show_bug.cgi?id=32264
+  apply_patch_with_msg \
+    3005-Make-.CRT-and-.idata-read-only.patch \
+    3006-Merge-.CRT-.ctors-and-.dtors-into-.rdata.patch
+
   # Add an option to change default bases back below 4GB to ease transition
   # https://github.com/msys2/MINGW-packages/issues/7027
   # https://github.com/msys2/MINGW-packages/issues/7023