XPost: linux.debian.bugs.dist
From: guillem@debian.org

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;
}

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)

XPost: linux.debian.bugs.dist
From: joshudson@gmail.com

Package: dpkg
Version: 1.21.22
Severity: important

Dear Maintainer,

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.

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.

This is a potential security vulnerability due to the bug class,
but I can'd find a plausible exploit pathway.

-- Package-specific info:
This system uses merged-usr-via-aliased-dirs, going behind dpkg's
back, breaking its core assumptions. This can cause silent file
overwrites and disappearances, and its general tools misbehavior.
See <https://wiki.debian.org/Teams/Dpkg/FAQ#broken-usrmerge>.

-- System Information:
Debian Release: 12.4
APT prefers stable-updates
APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 6.1.0-16-amd64 (SMP w/8 CPU threads; PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages dpkg depends on:
ii libbz2-1.0 1.0.8-5+b1
ii libc6 2.36-9+deb12u3
ii liblzma5 5.4.1-0.2
ii libmd0 1.0.4-2
ii libselinux1 3.4-1+b6
ii libzstd1 1.5.4+dfsg2-5
ii tar 1.34+dfsg-1.2
ii zlib1g 1:1.2.13.dfsg-1

dpkg recommends no packages.

Versions of packages dpkg suggests:
ii apt 2.6.1
pn debsig-verify <none>

-- no debconf information

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)

XPost: linux.debian.bugs.dist
From: joshudson@gmail.com

On Tue, Jan 23, 2024 at 3:16 PM Guillem Jover <guillem@debian.org> wrote:

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?

They were not. Must have been a 0 byte in the buffer after copyright.

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?

That patch fixed the bug. Knowing where the bug is, I can see how
the bug works and explain why. I'm wondering if this was just a
pending disaster for everybody or if there's some actual reason it
doesn't trip on official packages.

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)

XPost: linux.debian.bugs.dist
From: guillem@debian.org

Hi!

On Tue, 2024-01-23 at 16:47:34 -0800, Joshua Hudson wrote:

On Tue, Jan 23, 2024 at 3:16 PM Guillem Jover <guillem@debian.org> wrote:

On Tue, 2024-01-23 at 13:46:53 -0800, Joshua Hudson wrote:

Package: dpkg
Version: 1.21.22
Severity: important

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?

That patch fixed the bug. Knowing where the bug is, I can see how
the bug works and explain why. I'm wondering if this was just a
pending disaster for everybody or if there's some actual reason it
doesn't trip on official packages.

Thanks for testing! Much appreciated.

It looks like the code in libdpkg has been like that since long GNU
names and links were implemented (around Oct 1999, in dpkg commit <https://git.dpkg.org/cgit/dpkg/dpkg.git/commit/?id=3252594427f5285ab4091a6beca2adaa5082a883>).

Checking now the libdpkg test suite and the GNU tar implementation
which is what gets used to generate .debs by dpkg-deb, I see it always
includes the NUL byte as part of the size, which explains why this has
never been a problem when using the dpkg-deb tooling combined with GNU
tar.

<https://git.savannah.gnu.org/cgit/tar.git/tree/src/create.c#n542>

I assume, given the libtar link you provided initially, that your custom
.deb package is being generated by something using that library? If so
that would also explain things. I think libtar should probably get a
bug report to mimic the GNU behavior.

But regardless of libtar getting fixed or not, I'm still going to be
committing the change, to make the parser robust against such input.

Thanks,
Guillem

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)

XPost: linux.debian.bugs.dist
From: joshudson@gmail.com

On Tue, Jan 23, 2024 at 5:40 PM Guillem Jover <guillem@debian.org> wrote:

Hi!

On Tue, 2024-01-23 at 16:47:34 -0800, Joshua Hudson wrote:

On Tue, Jan 23, 2024 at 3:16 PM Guillem Jover <guillem@debian.org> wrote:

On Tue, 2024-01-23 at 13:46:53 -0800, Joshua Hudson wrote:

Package: dpkg
Version: 1.21.22
Severity: important

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?

That patch fixed the bug. Knowing where the bug is, I can see how
the bug works and explain why. I'm wondering if this was just a
pending disaster for everybody or if there's some actual reason it
doesn't trip on official packages.

Thanks for testing! Much appreciated.

It looks like the code in libdpkg has been like that since long GNU
names and links were implemented (around Oct 1999, in dpkg commit <https://git.dpkg.org/cgit/dpkg/dpkg.git/commit/?id=3252594427f5285ab4091a6beca2adaa5082a883>).

Checking now the libdpkg test suite and the GNU tar implementation
which is what gets used to generate .debs by dpkg-deb, I see it always includes the NUL byte as part of the size, which explains why this has
never been a problem when using the dpkg-deb tooling combined with GNU
tar.

<https://git.savannah.gnu.org/cgit/tar.git/tree/src/create.c#n542>

I assume, given the libtar link you provided initially, that your custom
.deb package is being generated by something using that library? If so
that would also explain things. I think libtar should probably get a
bug report to mimic the GNU behavior.

I used a couple of different sample sources to find how Gnu LongLinks work,
and yes libar was one of them. It looks like gnutar itself is the
oddball; everybody
else I found set the size to not include the trailing null. What's
funny is I thought
I had found that gnutar did the same; I guess I just misread the
hexdump of its output.
I guess that's what we all get for lack of an actual specification on it.

Incidentally, gnutar will read either way. Robustness is a good thing.

Here's another generator. Search for WriteAsGnu. Sorry I can't link to line because of new web accessibility issues on github.

https://raw.githubusercontent.com/dotnet/runtime/d250dcc2deae28fa9726ecad78dddf614b1420a8/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs

But regardless of libtar getting fixed or not, I'm still going to be committing the change, to make the parser robust against such input.

Much appreciated.

Thanks,
Guillem

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)