Hi!
On Tue, 2024-01-23 at 13:46:53 -0800, Joshua Hudson wrote:
> Package: dpkg
> Version: 1.21.22
> Severity: important
> On unpacking a custom .dpkg file with long symbolic links, I found a
> bunch of symbolic links ending in right, and one with copyright. The
> overrun made all the links exactly the same length; suggesting reuse
> of some kind of static buffer, but it's not clear if that's really
> the case.
>
> Making long link records an extra byte longer for the trailing null
> fixed the overrun and allowed the package to unpack correctly.
Where those long name lengths exactly multiples of 512?
> Source for long link record length does not include trailing null:
>
> https://repo.or.cz/libtar.git/blob/HEAD:/lib/block.c#l294
>
> I've stashed the offending .deb package but I'm not sure if I can
> get clearance to release it.
Ack. I did not try to reproduce this yet because it was not obvious
exactly how to do that from the report, instead just inspected the
code for potential brokenness related to this, and I think I've fixed
this now, but as I've not tested it, could you instead try applying
the attached patch against dpkg and test with your package whether
this fixes the problem you've found?
> This is a potential security vulnerability due to the bug class,
> but I can'd find a plausible exploit pathway.
I don't think this is a security issue, because dpkg-deb (which is the
only component that's expected to be used with untrusted .debs, in
contrast to dpkg which expects only trusted data) never uses the libdpkg
tar extracting implementation, and instead it uses the external GNU tar
implementation.
See <https://manpages.debian.org/unstable/dpkg/dpkg-deb#SECURITY>
and <https://manpages.debian.org/unstable/dpkg/dpkg#SECURITY>
for recently added blurbs on the security expectations. :)
Thanks,
Guillem
diff --git i/lib/dpkg/tarfn.c w/lib/dpkg/tarfn.c
index bc39acd7d..d999db68e 100644
--- i/lib/dpkg/tarfn.c
+++ w/lib/dpkg/tarfn.c
@@ -362,7 +362,7 @@ tar_gnu_long(struct tar_archive *tar, struct tar_entry *te, char **longp)
int long_read;
free(*longp);
- *longp = bp = m_malloc(te->size);
+ *longp = bp = m_malloc(te->size + 1);
for (long_read = te->size; long_read > 0; long_read -= TARBLKSZ) {
int copysize;
@@ -386,6 +386,7 @@ tar_gnu_long(struct tar_archive *tar, struct tar_entry *te, char **longp)
memcpy(bp, buf, copysize);
bp += copysize;
}
+ *bp = '\0';
return status;
}
