Re: ldd: check read return value to avoid unitialized struct fields

[Lucas]

Theo Buehler <t...@theobuehler.org> wrote:
> > -   if (pread(fd, phdr, size, ehdr.e_phoff) != size) {
> > +   if ((nr = pread(fd, phdr, size, ehdr.e_phoff)) != -1) {
> 
> did you intend to check for == -1?
> 
> >             warn("read(%s)", name);
> 
> should that not say pread?

Indeed, thanks for spotting both things.


-----------------------------------------------
commit 92f58b2a1cd576c3e72303004388ab1e9709e327 (ldd-read-rv)
from: Lucas <lucas@domain.invalid>
date: Sat Aug  5 16:34:16 2023 UTC
 
 Check {,p}read return values consistently
 
 Check that read performs a full header read. Explicitly check against -1
 for failure instead of < 0. Split pread error message between error
 handling and short reads. Promote size from int to size_t.
 
 M  libexec/ld.so/ldd/ldd.c

diff 194ff02fb6be247e27fe964e901d891d99ec0b74 
92f58b2a1cd576c3e72303004388ab1e9709e327
commit - 194ff02fb6be247e27fe964e901d891d99ec0b74
commit + 92f58b2a1cd576c3e72303004388ab1e9709e327
blob - 9e8c5065cd843ff36d91efcb868b94ffd4c98365
blob + 532feb9855a03480c289fa2188768657a4f82e7c
--- libexec/ld.so/ldd/ldd.c
+++ libexec/ld.so/ldd/ldd.c
@@ -96,7 +96,9 @@ doit(char *name)
 {
        Elf_Ehdr ehdr;
        Elf_Phdr *phdr;
-       int fd, i, size, status, interp=0;
+       size_t size;
+       ssize_t nr;
+       int fd, i, status, interp=0;
        char buf[PATH_MAX];
        struct stat st;
        void * dlhandle;
@@ -118,11 +120,16 @@ doit(char *name)
                return 1;
        }
 
-       if (read(fd, &ehdr, sizeof(ehdr)) < 0) {
+       if ((nr = read(fd, &ehdr, sizeof(ehdr))) == -1) {
                warn("read(%s)", name);
                close(fd);
                return 1;
        }
+       if (nr != sizeof(ehdr)) {
+               warnx("%s: incomplete ELF header", name);
+               close(fd);
+               return 1;
+       }
 
        if (!IS_ELF(ehdr) || ehdr.e_machine != ELF_TARG_MACH) {
                warnx("%s: not an ELF executable", name);
@@ -140,12 +147,18 @@ doit(char *name)
                err(1, "reallocarray");
        size = ehdr.e_phnum * sizeof(Elf_Phdr);
 
-       if (pread(fd, phdr, size, ehdr.e_phoff) != size) {
-               warn("read(%s)", name);
+       if ((nr = pread(fd, phdr, size, ehdr.e_phoff)) == -1) {
+               warn("pread(%s)", name);
                close(fd);
                free(phdr);
                return 1;
        }
+       if (nr != size) {
+               warnx("%s: incomplete program header", name);
+               close(fd);
+               free(phdr);
+               return 1;
+       }
        close(fd);
 
        for (i = 0; i < ehdr.e_phnum; i++)