Repository: NixOS/patchelf (language: python)

Issue:
Cannot add-needed and replace-needed in same invocation
**Describe the bug**

I cannot `--add-needed` and `--replace-needed` on the same invocation.
It produces a binary with incorrect `DT_NEEDED` and `gnu.version_r` sections.

**Steps To Reproduce**
```bash
❯ cp /usr/bin/ruby /tmp/ruby

❯ ./src/patchelf --print-needed /tmp/ruby
libruby-2.7.so.2.7
libc.so.6

❯ ./src/patchelf --replace-needed libruby-2.7.so.2.7 /lib/x86_64-linux-gnu/libruby-2.7.so.2.7 \
--replace-needed libc.so.6 /lib/x86_64-linux-gnu/libc.so.6 \
--add-needed /lib/x86_64-linux-gnu/libcrypt.so.1 \
--add-needed /lib/x86_64-linux-gnu/libdl.so.2 \
--add-needed /lib/x86_64-linux-gnu/libgmp.so.10 \
--add-needed /lib/x86_64-linux-gnu/libm.so.6 \
--add-needed /lib/x86_64-linux-gnu/libpthread.so.0 \
--add-needed /lib/x86_64-linux-gnu/librt.so.1 /tmp/ruby

❯ ./src/patchelf --print-needed /tmp/ruby
/lib/x86_64-linux-gnu/libcrypt.so.1
/lib/x86_64-linux-gnu/libdl.so.2
/lib/x86_64-linux-gnu/libgmp.so.10
/lib/x86_64-linux-gnu/libm.so.6
/lib/x86_64-linux-gnu/libpthread.so.0
/lib/x86_64-linux-gnu/librt.so.1
/lib/x86_64-linux-gnu/libcrypt.so.1
x86_64-linux-gnu/libdl.so.2
```

Please see that there are **multiple** copies of `libcrypt.so.1` and `libdl.so.2`, the last one is not even a full path.

The `.gnu.version_r` also looks incorrect. It is now `x86_64-linux-gnu/libdl.so.2` instead of `glibc`.
```
❯ readelf -a /tmp/ruby | grep -A 5 gnu.version_r
  [ 3] .gnu.version_r    VERNEED          00000000000006e0  000006e0
       0000000000000030  0000000000000000   A      22     1     8
  [ 4] .rela.dyn         RELA             0000000000000710  00000710
       00000000000000c0  0000000000000018   A       1     0     8
  [ 5] .rela.plt         RELA             00000000000007d0  000007d0
       00000000000000a8  0000000000000018  AI       1    16     8
--
   01     .dynsym .gnu.version .gnu.version_r .rela.dyn .rela.plt 
   02     
   03     .init .plt .plt.got .text .fini 
   04     .rodata .eh_frame_hdr .eh_frame 
   05     .eh_frame_hdr 
   06     .init_array .fini_array .got .data .bss 
--
Version needs section '.gnu.version_r' contains 1 entry:
 Addr: 0x00000000000006e0  Offset: 0x0006e0  Link: 22 (.dynstr)
  000000: Version: 1  File: x86_64-linux-gnu/libdl.so.2  Cnt: 2
  0x0010:   Name: GLIBC_2.4  Flags: none  Version: 3
  0x0020:   Name: GLIBC_2.2.5  Flags: none  Version: 2
```

**Expected behavior**

I expect the `--print-needed` to reflect what was passed in.

**`patchelf --version` output**

```
❯ ./src/patchelf --version
patchelf 0.14.3
```

**Additional context**

I discovered this also when trying to do the same in _patchelf_ itself via https://github.com/NixOS/patchelf/pull/357


Discussion / hints:

Fix patch:
diff --git a/.gitignore b/.gitignore
index 921b9f85..c3f60a5d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,3 +36,4 @@ Makefile
 .direnv/
 .vscode/
 .idea/
+result
diff --git a/src/patchelf.cc b/src/patchelf.cc
index 668a5e49..e824cbf9 100644
--- a/src/patchelf.cc
+++ b/src/patchelf.cc
@@ -1150,6 +1150,7 @@ void ElfFile<ElfFileParamNames>::modifySoname(sonameMode op, const std::string &
     }
 
     changed = true;
+    this->rewriteSections();
 }
 
 template<ElfFileParams>
@@ -1158,6 +1159,7 @@ void ElfFile<ElfFileParamNames>::setInterpreter(const std::string & newInterpret
     std::string & section = replaceSection(".interp", newInterpreter.size() + 1);
     setSubstr(section, 0, newInterpreter + '\0');
     changed = true;
+    this->rewriteSections();
 }
 
 
@@ -1234,6 +1236,7 @@ void ElfFile<ElfFileParamNames>::removeRPath(Elf_Shdr & shdrDynamic) {
         }
     }
     memset(last, 0, sizeof(Elf_Dyn) * (dyn - last));
+    this->rewriteSections();
 }
 
 template<ElfFileParams>
@@ -1380,6 +1383,7 @@ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
         newDyn.d_un.d_val = shdrDynStr.sh_size;
         setSubstr(newDynamic, 0, std::string((char *) &newDyn, sizeof(Elf_Dyn)));
     }
+    this->rewriteSections();
 }
 
 
@@ -1409,6 +1413,8 @@ void ElfFile<ElfFileParamNames>::removeNeeded(const std::set<std::string> & libs
     }
 
     memset(last, 0, sizeof(Elf_Dyn) * (dyn - last));
+
+    this->rewriteSections();
 }
 
 template<ElfFileParams>
@@ -1532,6 +1538,8 @@ void ElfFile<ElfFileParamNames>::replaceNeeded(const std::map<std::string, std::
             --verNeedNum;
         }
     }
+
+    this->rewriteSections();
 }
 
 template<ElfFileParams>
@@ -1582,6 +1590,8 @@ void ElfFile<ElfFileParamNames>::addNeeded(const std::set<std::string> & libs)
     }
 
     changed = true;
+
+    this->rewriteSections();
 }
 
 template<ElfFileParams>
@@ -1638,6 +1648,7 @@ void ElfFile<ElfFileParamNames>::noDefaultLib()
         setSubstr(newDynamic, 0, std::string((char *) &newDyn, sizeof(Elf_Dyn)));
     }
 
+    this->rewriteSections();
     changed = true;
 }
 
@@ -1667,6 +1678,7 @@ void ElfFile<ElfFileParamNames>::clearSymbolVersions(const std::set<std::string>
         }
     }
     changed = true;
+    this->rewriteSections();
 }
 
 static bool printInterpreter = false;
@@ -1726,7 +1738,6 @@ static void patchElf2(ElfFile && elfFile, const FileContents & fileContents, con
         elfFile.noDefaultLib();
 
     if (elfFile.isChanged()){
-        elfFile.rewriteSections();
         writeFile(fileName, elfFile.fileContents);
     } else if (alwaysWrite) {
         debug("not modified, but alwaysWrite=true\n");


Test patch:
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 404f159d..b280e62d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -39,7 +39,8 @@ src_TESTS = \
   basic-flags.sh \
   set-empty-rpath.sh \
   phdr-corruption.sh \
-  replace-needed.sh
+  replace-needed.sh \
+  replace-add-needed.sh
 
 build_TESTS = \
   $(no_rpath_arch_TESTS)
diff --git a/tests/replace-add-needed.sh b/tests/replace-add-needed.sh
new file mode 100755
index 00000000..ab0d3532
--- /dev/null
+++ b/tests/replace-add-needed.sh
@@ -0,0 +1,38 @@
+#! /bin/sh -e
+SCRATCH=scratch/$(basename $0 .sh)
+PATCHELF=$(readlink -f "../src/patchelf")
+
+rm -rf ${SCRATCH}
+mkdir -p ${SCRATCH}
+
+cp simple ${SCRATCH}/
+cp libfoo.so ${SCRATCH}/
+cp libbar.so ${SCRATCH}/
+
+cd ${SCRATCH}
+
+libcldd=$(ldd ./simple | awk '/ => / { print $3 }' | grep -E "(libc.so|ld-musl)")
+
+# We have to set the soname on these libraries
+${PATCHELF} --set-soname libbar.so ./libbar.so
+
+# Add a libbar.so so we can rewrite it later
+${PATCHELF} --add-needed libbar.so ./simple
+
+# Make the NEEDED in libfoo the same as simple
+# This is a current "bug" in musl
+# https://www.openwall.com/lists/musl/2021/12/21/1
+${PATCHELF} --replace-needed libbar.so $(readlink -f ./libbar.so) ./libfoo.so
+
+${PATCHELF} --replace-needed libc.so.6 ${libcldd} \
+            --replace-needed libbar.so $(readlink -f ./libbar.so) \
+            --add-needed $(readlink -f ./libfoo.so) \
+            ./simple
+
+exitCode=0
+./simple || exitCode=$?
+
+if test "$exitCode" != 0; then
+    ldd ./simple
+    exit 1
+fi
\ No newline at end of file