Package: chrpath
Version: 0.16-2
Severity: important
Tags: patch upstream
Dear Maintainer,
I'm forwarding this analysis and patch from a corresponding Fedora report
(https://bugzilla.redhat.com/show_bug.cgi?id=2022931), as we have the same
unreachable Debian URL listed as upstream
(https://alioth.debian.org/projects/chrpath/).
I first encountered and diagnosed this issue on a Fedora install, but have
since verified chrpath in Ubuntu 20.04 (and by extension, Debian) is affected
by the same bug. The steps to verify (below) are slightly different, but the
outcome is the same.
My patch to fix the bug (submitted to Fedora in
https://src.fedoraproject.org/rpms/chrpath/pull-request/6, but also included
below) should be applicable as well.
The issue arises when chrpath is asked to read or modify an ELF binary where
the .dynstr header section (into which the offset of the RUNPATH is recorded)
is NOT the first header section of type SHT_STRTAB in the headers. When reading
or modifying an ELF binary, chrpath uses the first SHT_STRTAB section it finds
as the symbol table to modify, without verifying that it is the correct .dynstr
table.
Binaries with a .dynstr header section located AFTER other header sections of
the same type will be created when using the patchelf tool to add a RUNPATH to
a library which was originally built without one, though that is likely not the
only possible means of creating such a file.
To reproduce with current chrpath, patchelf, binutils (for readelf), coreutils
(for realpath), libc-bin (for ldd), and gcc packages installed (I used a Ubuntu
20.04 VM with gcc 9):
PREPARATION
1. Create a file library.c with the following contents:
$ cd /var/tmp # (any writable directory will do)
$ cat > library.c
#include <stdio.h>
void test_lib(void) {
printf("%s\n", "Hello, shared library");
}
^D
2. Compile as a shared library 'library2.so':
$ gcc -shared -fPIC -o library2.so library.c
3. Compile a second time as 'library.so':
$ gcc -shared -fPIC -o library.so library.c
4. Make 'library.so' dynamically link to 'library2.so':
$ patchelf --add-needed library2.so library.so
$ ldd library.so
linux-vdso.so.1 (0x00007ffed6781000)
library2.so => not found
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f08fc8b5000)
/lib64/ld-linux-x86-64.so.2 (0x00007f08fcad6000)
5. Use 'patchelf' to add a RUNPATH so that one library can find the other:
$ patchelf --print-rpath library.so
$ patchelf --set-rpath $(realpath .) library.so
$ patchelf --print-rpath library.so
/var/tmp
$ ldd library.so
linux-vdso.so.1 (0x00007ffca37f9000)
library2.so => /var/tmp/library2.so (0x00007f788f39d000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f788f184000)
/lib64/ld-linux-x86-64.so.2 (0x00007f788f3ab000)
REPRODUCTION
If 'chrpath' is now used on our modified 'library.so', bad things happen:
$ chrpath -l library.so
library.so: RUNPATH=_init_array_entry
$ chrpath -r '/tmp' library.so
library.so: RUNPATH=_init_array_entry
library.so: new RUNPATH: /tmp
$ ldd library.so
linux-vdso.so.1 (0x00007ffcfc775000)
library2.so => /var/tmp/library2.so (0x00007facbc55d000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007facbc344000)
/lib64/ld-linux-x86-64.so.2 (0x00007facbc56b000)
$ readelf -d library.so|grep PATH
0x000000000000001d (RUNPATH) Library runpath: [/var/tmp]
$ nm library.so
0000000000004028 b completed.8061
w __cxa_finalize@@GLIBC_2.2.5
0000000000001060 t deregister_tm_clones
00000000000010d0 t __do_global_dtors_aux
0000000000003e18 d __do_global_dtors_aux_fini_array_entry
0000000000004020 d __dso_handle
0000000000003e20 d _DYNAMIC
0000000000001130 t _fini
0000000000001110 t frame_dummy
0000000000003e10 d __frame_dummy/tmp
00000000000020d8 r __FRAME_END__
0000000000004000 d _GLOBAL_OFFSET_TABLE_
w __gmon_start__
0000000000002018 r __GNU_EH_FRAME_HDR
0000000000001000 t _init
w _ITM_deregisterTMCloneTable
w _ITM_registerTMCloneTable
U puts@@GLIBC_2.2.5
0000000000001090 t register_tm_clones
0000000000001119 T test_lib
0000000000004028 d __TMC_END__
Because it interpreted the offset value 'rpathoff' as an offset into the wrong
header section (the '.strtab' table, rather than the '.dynstr' table),
'chrpath' wrote its new RUNPATH value in completely the wrong location in the
header, partially corrupting the name of a different symbol entirely.
The attached patch, against the last available chrpath-0.16 source before the
upstream host went dark, corrects this issue and makes 'chrpath' safe to run on
'patchelf'-modified libraries. Details of the fix are included in the commit
message.
-- System Information:
Debian Release: bullseye/sid
APT prefers focal-updates
APT policy: (500, 'focal-updates'), (500, 'focal-security'), (500, 'focal')
Architecture: amd64 (x86_64)
Foreign Architectures: i386
Kernel: Linux 5.4.0-104-generic (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8),
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
Versions of packages chrpath depends on:
ii libc6 2.31-0ubuntu9.9
chrpath recommends no packages.
chrpath suggests no packages.
>From 389098510c9edc4020ac9fb38c7ed6027848426f Mon Sep 17 00:00:00 2001
From: "FeRD (Frank Dana)" <ferd...@gmail.com>
Date: Sun, 24 Apr 2022 19:12:19 -0400
Subject: [PATCH] Fix lookup of string table
Previously, the code assumed the first SHT_STRTAB section it
encountered was the dynamic string table. However, this is not
always the case.
Two new functions, read_section_names() and get_section_name(),
are responsible for reading the table of section names from the
ELF file, and retrieving section names from the imported table.
The search for the dynamic string table is now augmented by a
check that the section name is ".dynstr", before applying the
rpath offset to retrieve the rpath string.
---
chrpath.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
protos.h | 3 +++
2 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/chrpath.c b/chrpath.c
index 207e369..06920a8 100644
--- a/chrpath.c
+++ b/chrpath.c
@@ -50,6 +50,61 @@ Peeter
#include <sys/stat.h>
#include "protos.h"
+
+/**
+ * Reads the section names table from an ELF file into memory
+ */
+char*
+read_section_names(int fd, Elf_Ehdr ehdr)
+{
+ Elf_Shdr shdr;
+
+ const size_t sz_shdr = is_e32() ? sizeof(Elf32_Shdr) : sizeof(Elf64_Shdr);
+ const size_t sh_off = EHDRWU(e_shoff);
+ const size_t sections_off = EHDRWU(e_shstrndx) * sz_shdr;
+
+ if (lseek(fd, sh_off + sections_off, SEEK_SET) == -1)
+ {
+ perror ("positioning for section table");
+ return NULL;
+ }
+
+ if (read(fd, &shdr, sz_shdr) != (ssize_t)sz_shdr)
+ {
+ perror ("reading section header");
+ return NULL;
+ }
+
+ /* +1 for forced trailing null */
+ char* strtab = (char *)calloc(1, SHDR_O(sh_size)+1);
+ if (strtab == NULL)
+ {
+ perror ("allocating memory for string table");
+ return NULL;
+ }
+
+ if (lseek(fd, SHDR_O(sh_offset), SEEK_SET) == -1)
+ {
+ perror ("positioning for string table");
+ return NULL;
+ }
+ if (read(fd, strtab, SHDR_O(sh_size)) != (ssize_t)SHDR_O(sh_size))
+ {
+ perror ("reading string table");
+ return NULL;
+ }
+ strtab[SHDR_O(sh_size)] = 0; /* make sure printed string is null terminated
*/
+ return strtab;
+}
+
+/**
+ * Looks up the name for a section, using its offset in the table
+ */
+char*
+get_section_name(int name_off, char* section_names) {
+ return section_names + name_off;
+}
+
/**
* Reads an ELF file, and reads or alters the RPATH setting.
*
@@ -128,31 +183,44 @@ chrpath(const char *filename, const char *newpath, int
convert)
return 2;
}
+ char* section_names = read_section_names(fd, ehdr);
+ if (section_names == NULL) {
+ free(dyns);
+ elf_close(fd);
+ return 1;
+ }
+
if (lseek(fd, EHDRWU(e_shoff), SEEK_SET) == -1)
{
perror ("positioning for sections");
free(dyns);
+ free(section_names);
elf_close(fd);
return 1;
}
+ const size_t sz_shdr = is_e32() ? sizeof(Elf32_Shdr) : sizeof(Elf64_Shdr);
for (i = 0; i < EHDRHU(e_shnum); i++)
{
- const size_t sz_shdr = is_e32() ? sizeof(Elf32_Shdr) : sizeof(Elf64_Shdr);
if (read(fd, &shdr, sz_shdr) != (ssize_t)sz_shdr)
{
perror ("reading section header");
free(dyns);
+ free(section_names);
elf_close(fd);
return 1;
}
- if (SHDR_W(sh_type) == SHT_STRTAB)
+ if (SHDR_W(sh_type) != SHT_STRTAB)
+ continue;
+ char* name = get_section_name(SHDR_W(sh_name), section_names);
+ if (strcmp(name, ".dynstr") == 0)
break;
}
if (i == EHDRHU(e_shnum))
{
fprintf (stderr, "No string table found.\n");
free(dyns);
+ free(section_names);
elf_close(fd);
return 2;
}
@@ -162,6 +230,7 @@ chrpath(const char *filename, const char *newpath, int
convert)
{
perror ("allocating memory for string table");
free(dyns);
+ free(section_names);
elf_close(fd);
return 1;
}
@@ -171,6 +240,7 @@ chrpath(const char *filename, const char *newpath, int
convert)
perror ("positioning for string table");
free(strtab);
free(dyns);
+ free(section_names);
elf_close(fd);
return 1;
}
@@ -179,6 +249,7 @@ chrpath(const char *filename, const char *newpath, int
convert)
perror ("reading string table");
free(strtab);
free(dyns);
+ free(section_names);
elf_close(fd);
return 1;
}
@@ -190,6 +261,7 @@ chrpath(const char *filename, const char *newpath, int
convert)
elf_tagname(DYNSS(rpath_dyns_index, d_tag)));
free(strtab);
free(dyns);
+ free(section_names);
elf_close(fd);
return 5;
}
diff --git a/protos.h b/protos.h
index f24bc06..c6af396 100644
--- a/protos.h
+++ b/protos.h
@@ -55,6 +55,9 @@ int swap_bytes(void);
int killrpath(const char *filename);
int chrpath(const char *filename, const char *newpath, int convert);
+char* read_section_names(int fd, Elf_Ehdr ehdr);
+char* get_section_name(int name_off, char* section_names);
+
int elf_open(const char *filename, int flags, Elf_Ehdr *ehdr);
void elf_close(int fd);
int elf_find_dynamic_section(int fd, Elf_Ehdr *ehdr, Elf_Phdr *phdr);
--
2.35.1
