Debian Bug report logs -
#225692
dpkg: Allows users to stash away vulnerable versions of setuid binaries
Reported by: Erno Kuusela <erno-debbugs@erno.iki.fi>
Date: Wed, 31 Dec 2003 17:48:10 UTC
Severity: grave
Tags: fixed, patch
Found in version 1.9.21
Fixed in version dpkg/1.10.19
Done: Scott James Remnant <scott@netsplit.com>
Bug is archived. No further changes may be made.
Toggle useless messages
Report forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Erno Kuusela <erno-debbugs@erno.iki.fi>:
New Bug report received and forwarded. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #5 received at submit@bugs.debian.org (full text, mbox, reply):
Package: dpkg
Version: 1.9.21
Severity: grave
Tags: security
see http://lists.jammed.com/ISN/2003/12/0056.html
users can make hardlinks to root owned setuid binaries in the usual
partitioning configurations, so unlinking them is not a reliable way
to get rid of them.
with the current dpkg behaviour it's not enough to upgrade the package
before malicious local users get their hands on the exploit, since they
can stash the binary away and wait for an exploit to become available.
i think a fix for this might be to open() the binary, unlink() it,
and then use fchmod() to remove the setuid and setgid bits.
(optionally check link count to see if someone is trying this
and print a warning)
truncate() is out since running copies of the binaries
won't like it, and a normal chmod() would be racy...
-- System Information:
Debian Release: 3.0
Architecture: i386
Kernel: Linux fabulous 2.6.0 #2 Sun Dec 21 10:27:12 EET 2003 i686
Locale: LANG=C, LC_CTYPE=fi_FI
Versions of packages dpkg depends on:
ii libc6 2.3.2.ds1-10 GNU C Library: Shared libraries an
ii libncurses5 5.3.20030719-2 Shared libraries for terminal hand
ii libstdc++2.10-glibc2.2 1:2.95.4-15 The GNU stdc++ library
-- no debconf information
Message sent on to Erno Kuusela <erno-debbugs@erno.iki.fi>:
Bug#225692.
(full text, mbox, link).
Message #8 received at 225692-submitter@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
This is really a common setuid problem, and, just for reference the
article you mention was published after a contest:
Contest - The mysteriously persistently exploitable program.
http://www.hackinglinuxexposed.com/articles/20031111.html
(Notice that the incident is Debian-related)
The mysteriously persistently exploitable program explained.
http://www.hackinglinuxexposed.com/articles/20031214.html
I'm not sure if this bug should qualify as 'grave' since it's not dpkg
task to control who symlinks to potentially dangerous binaries. As
described in the Securing Debian Manual (Mounting partitions the right way
[1]) it is the administrator task to avoid symlink attacks (as well as DoS
attacks due to system partitions filling up) by separating user-writable
directories (these include /home, /tmp and /var/tmp). These directories
should be nosuid, and nodev (and maybe noexec too even though it provides
little protection).
Notably, though, I just reviewed dpkg's code, and found this:
(dpkg-1.10.18/main/remove.c)
259 {
260 /*
261 * If file to remove is a device or s[gu]id, change its mode
262 * so that a malicious user cannot use it even if it's linked
263 * to another file
264 */
265 struct stat stat_buf;
266 if (stat(fnvb.buf,&stat_buf)==0) {
267 if (S_ISCHR(stat_buf.st_mode) || S_ISBLK(stat_buf.st_mode)) {
268 chmod(fnvb.buf,0);
269 }
270 if (stat_buf.st_mode & (S_ISUID|S_ISGID)) {
271 chmod(fnvb.buf,stat_buf.st_mode & ~(S_ISUID|S_ISGID));
272 }
273 }
274 }
275 if (unlink(fnvb.buf)) ohshite(_("cannot remove file `%.250s'"),fnv 275 b.buf);
Now, I believe this is done when removing a package (it's under
removal_bulk()), but _not_ when upgrading (is it done with
process_archive()):
dpkg-1.10.18/main/processarc.c
/*
* Now we unpack the archive, backing things up as we go.
* For each file, we check to see if it already exists.
* There are several possibilities:
* + We are trying to install a non-directory ...
* - It doesn't exist. In this case we simply extract it.
* - It is a plain file, device, symlink, &c. We do an `atomic
* overwrite' using link() and rename(), but leave a backup copy.
* Later, when we delete the backup, we remove it from any other
* packages' lists.
As far as I understand the code (which I might get it wrong), it will just
get unlinked:
641: if (donotrm) continue;
642: if (!unlink(fnamevb.buf)) continue;
and is not modified as per removal.
Shouldn't it be better if the code used in removal.c was re-used in
processarc.c. As I understand it, dpkg is vulnerable to symlink attacks to
suid binaries _only_ when a package is upgraded but not when it's removed.
If my assessment is accurate maybe this bug could be tagged 'patch' by the
dpkg team (the code from lines 259-274 of remove.c should be copied
verbatim between lines 641 and 642 of processarc and probably then
's/fnvb/fnamevb'). In any case IMHO this bug should be re-prioried as
'wishlist' (but keeping the security tag).
Regards
Javier Fernandez-Sanguino
[1]
http://www.debian.org/doc/manuals/securing-debian-howto/ch4.en.html#s4.9
[signature.asc (application/pgp-signature, inline)]
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Erno Kuusela <erno@iki.fi>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #13 received at 225692@bugs.debian.org (full text, mbox, reply):
hello,
the issue is specifically hard links, there is no problem with symlinks.
| I'm not sure if this bug should qualify as 'grave' since it's not dpkg
| task to control who symlinks to potentially dangerous binaries. As
no, but dpkg could handle the upgrade / safe neutralization of old setuid
binaries in the manner i described, and it doesn't.
| described in the Securing Debian Manual (Mounting partitions the right way
| [1]) it is the administrator task to avoid symlink attacks (as well as DoS
| attacks due to system partitions filling up) by separating user-writable
| directories (these include /home, /tmp and /var/tmp). These directories
| should be nosuid, and nodev (and maybe noexec too even though it provides
| little protection).
then the installer should make sure the system gets partitioned and
configured this way, or warn the user in big friendly letters. but
solving the problem with partitions is not as good solution in my
opinion, since fragmenting disks to multiple partitions can lead to
inflexibility and other problems.
the rest of your mail regarding dpkg code looks good to me although
i'm no expert on dpkg.
-- erno
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Erno Kuusela <erno@iki.fi>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #18 received at 225692@bugs.debian.org (full text, mbox, reply):
| Shouldn't it be better if the code used in removal.c was re-used in
after turning my brain on i remember what my original point about
needing to use fchmod was... the usual way to upgrade binaries in unix
is to use link() or rename() to replace them atomically. there also you
can use fchmod to change the permissions of the old inode (which might
still other links). if you just chmod the setuid bit away before doing
the replacement, there's window of time where you have a nonfunctional
binary in place.
-- erno
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Javier Fernández-Sanguino Peña <jfs@computer.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #23 received at 225692@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
On Sun, Jan 04, 2004 at 03:24:09AM +0200, Erno Kuusela wrote:
> hello,
>
> the issue is specifically hard links, there is no problem with symlinks.
Sorry, I meant hard links [1]
>
> | I'm not sure if this bug should qualify as 'grave' since it's not dpkg
> | task to control who symlinks to potentially dangerous binaries. As
>
> no, but dpkg could handle the upgrade / safe neutralization of old setuid
> binaries in the manner i described, and it doesn't.
Still, it's a wishlist bug, you are asking for an improvement to solve a
security situation.
>
> | described in the Securing Debian Manual (Mounting partitions the right way
> | [1]) it is the administrator task to avoid symlink attacks (as well as DoS
> | attacks due to system partitions filling up) by separating user-writable
> | directories (these include /home, /tmp and /var/tmp). These directories
> | should be nosuid, and nodev (and maybe noexec too even though it provides
> | little protection).
>
> then the installer should make sure the system gets partitioned and
> configured this way, or warn the user in big friendly letters. but
> solving the problem with partitions is not as good solution in my
> opinion, since fragmenting disks to multiple partitions can lead to
> inflexibility and other problems.
Notice that proper partitions _are_ one way to fix this issue [2]. Even if
you fix dpkg you are still prone to DoS attacks and hardlink attacks to
local binaries (/usr/local) not handled by dpkg (or even by installation of
local binaries if you do it in /usr/ but do not use debian packages)
>
> the rest of your mail regarding dpkg code looks good to me although
> i'm no expert on dpkg.
I'm not either :-)
Javi
[1] This is a "UNIX feature" BTW.
Sample references include:
http://lists.insecure.org/lists/vuln-dev/1999/Dec/0027.html
and
http://cr.yp.to/maildisasters/postfix.19981221 (see Technical Notes)
and
http://www.cs.uml.edu/~acahalan/linux/obstacles.html
and
http://www.ussg.iu.edu/hypermail/linux/kernel/9612.1/0378.html
[2] Another way to fix this issue is doing it on the kernel, like Openwall
does: http://www.openwall.com/linux/README.shtml (see "Restricted links in
/tmp.")
[signature.asc (application/pgp-signature, inline)]
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Erno Kuusela <erno@iki.fi>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #28 received at 225692@bugs.debian.org (full text, mbox, reply):
hello,
On Sun, 04 Jan 2004, Javier Fernández-Sanguino Peña wrote:
| > | I'm not sure if this bug should qualify as 'grave' since it's not dpkg
| > | task to control who symlinks to potentially dangerous binaries. As
| >
| > no, but dpkg could handle the upgrade / safe neutralization of old setuid
| > binaries in the manner i described, and it doesn't.
|
| Still, it's a wishlist bug, you are asking for an improvement to solve a
| security situation.
well, the current situation yields local root vulnerabilities, thus it
is clearly a bug in the system. from my laymans point of view this is
dpkg's responsibility since it's replacing the binaries on upgrade.
i don't see why it would be a "wishlist" bug.
| Notice that proper partitions _are_ one way to fix this issue [2]. Even if
| you fix dpkg you are still prone to DoS attacks and hardlink attacks to
| local binaries (/usr/local) not handled by dpkg (or even by installation of
| local binaries if you do it in /usr/ but do not use debian packages)
i guess install(1) should be fixed as well. /usr/local stuff and other
third party stuff is beyond debian's control anyhow, responsibility
is on the third party packages.
[from the next mail]
| Notice that my proposal is to chmod the setuid bit just before it's
| unlinked in the dpkg code. When a binary is substituted it's first renamed
| (link/rename) and then removed, the chmod bit should be removed before the
| file itself is removed
ah ok, if you do
"ln foo foo.old && mv foo.new foo && chmod -s foo.old && rm foo.old",
that works too.
-- erno
Tags removed: security
Request was from Matt Zimmerman <mdz@debian.org>
to control@bugs.debian.org.
(full text, mbox, link).
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Javier Fernández-Sanguino Peña <jfs@computer.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #35 received at 225692@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
On Sun, Jan 04, 2004 at 05:58:08AM +0200, Erno Kuusela wrote:
> | Shouldn't it be better if the code used in removal.c was re-used in
>
> after turning my brain on i remember what my original point about
> needing to use fchmod was... the usual way to upgrade binaries in unix
> is to use link() or rename() to replace them atomically. there also you
> can use fchmod to change the permissions of the old inode (which might
> still other links). if you just chmod the setuid bit away before doing
> the replacement, there's window of time where you have a nonfunctional
> binary in place.
Notice that my proposal is to chmod the setuid bit just before it's
unlinked in the dpkg code. When a binary is substituted it's first renamed
(link/rename) and then removed, the chmod bit should be removed before the
file itself is removed IMHO, that shouldn't result on having a
nonfunctional binary.
Regards
Javi
[signature.asc (application/pgp-signature, inline)]
Tags added: patch
Request was from Javier Fernández-Sanguino Peña <jfs@computer.org>
to control@bugs.debian.org.
(full text, mbox, link).
Message sent on to Erno Kuusela <erno-debbugs@erno.iki.fi>:
Bug#225692.
(full text, mbox, link).
Message #40 received at 225692-submitter@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
tags 225692 patch
thanks
The following (yet untested) patch should provide the same mechanism for
upgrades as for removes in order to avoid the hard link attacks described
in the bug report.
I will try to test this in a limited environment to see if it fulfills the
needs (avoids these attacks) but wider testing would be good.
Regards
Javi
[stash-suid-fix.diff (text/plain, attachment)]
[signature.asc (application/pgp-signature, inline)]
Message sent on to Erno Kuusela <erno-debbugs@erno.iki.fi>:
Bug#225692.
(full text, mbox, link).
Message #43 received at 225692-submitter@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
Just a note to let bug readers now that the bug works as expected OK when
upgrading packages. I've just tried in a minimal (debootsrap) sid
environment to upgrade from woody's to sid's pppd version (script below).
Notice this is minimal testing, I would prefer if other people test the
change and reviewed the code to ensure that it won't introduce any bugs.
Regards
Javi
PS: If anyone wants pre-compiled binaries/packages, please ask.
[ As root ]
# dpkg -i dpkg_1.10.18.1_i386.deb
(Reading database ... 7293 files and directories currently installed.)
Preparing to replace dpkg 1.10.18 (using dpkg_1.10.18.1_i386.deb) ...
Unpacking replacement dpkg ...
Setting up dpkg (1.10.18.1) ...
# dpkg -i ppp_2.4.2+20031002-4_i386.deb
Preparing to replace ppp 2.4.2+20031002-4 (using ppp_2.4.2+20031002-4_i386.deb) ...
Unpacking replacement ppp ...
Setting up ppp (2.4.2+20031002-4) ...
[ As a rogue user ]
test@localhost:/tmp$ ln /usr/sbin/pppd ..pppd_vulnerable
test@localhost:/tmp$ ls -la ..pp*
-rwsr-xr-- 2 root dip 277624 Oct 30 21:44 ..pppd_vulnerable
[ Root upgrades the package ]
# dpkg -i ppp_2.4.2+20031127-2_i386.deb
(Reading database ... 7293 files and directories currently installed.)
Preparing to replace ppp 2.4.2+20031002-4 (using
ppp_2.4.2+20031127-2_i386.deb) ...
Unpacking replacement ppp ...
Setting up ppp (2.4.2+20031127-2) ...
Installing new version of config file /etc/ppp/ip-up.d/0000usepeerdns ...
Installing new version of config file /etc/apm/event.d/ppp ...
# md5sum /usr/sbin/pppd
b70e5c63b5080b2701921a5e42669f73 /usr/sbin/pppd
# md5sum /tmp/..pppd_vulnerable
c32f54f2021e7fcbf9c033d1866a4766 /tmp/..pppd_vulnerable
# ls -la /tmp/..pppd_vulnerable
-rw------- 1 root dip 277624 Oct 30 21:44 /tmp/..pppd_vulnerable
Which renders the vulnerable hardlink copy of the binary useless to the
attacker.
[signature.asc (application/pgp-signature, inline)]
Acknowledgement sent to Erno Kuusela <erno@iki.fi>:
Extra info received and filed, but not forwarded.
(full text, mbox, link).
Message #48 received at 225692-quiet@bugs.debian.org (full text, mbox, reply):
hello,
On Wed, 07 Jan 2004, Javier Fernández-Sanguino Peña wrote:
| Just a note to let bug readers now that the bug works as expected OK when
| upgrading packages. I've just tried in a minimal (debootsrap) sid
| environment to upgrade from woody's to sid's pppd version (script below).
cool. i hope there will be a security update for woody?
| Notice this is minimal testing, I would prefer if other people test the
| change and reviewed the code to ensure that it won't introduce any bugs.
| PS: If anyone wants pre-compiled binaries/packages, please ask.
i am willing to test if you have ready to go packages.
-- erno
Acknowledgement sent to Javier Fernández-Sanguino Peña <jfs@computer.org>:
Extra info received and filed, but not forwarded.
(full text, mbox, link).
Message #53 received at 225692-quiet@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
On Sat, Jan 10, 2004 at 05:57:47PM +0200, Erno Kuusela wrote:
>
> i am willing to test if you have ready to go packages.
Available now at people.debian.org/~jfs/dpkg
Javi
[signature.asc (application/pgp-signature, inline)]
Acknowledgement sent to Erno Kuusela <erno@iki.fi>:
Extra info received and filed, but not forwarded.
(full text, mbox, link).
Message #58 received at 225692-quiet@bugs.debian.org (full text, mbox, reply):
hello,
On Mon, 12 Jan 2004, Javier Fernández-Sanguino Peña wrote:
| On Sat, Jan 10, 2004 at 05:57:47PM +0200, Erno Kuusela wrote:
| >
| > i am willing to test if you have ready to go packages.
|
| Available now at people.debian.org/~jfs/dpkg
seems to work as advertised.
-- erno
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Erno Kuusela <erno@iki.fi>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #63 received at 225692@bugs.debian.org (full text, mbox, reply):
On Sun, 18 Jan 2004, Erno Kuusela wrote:
| | Available now at people.debian.org/~jfs/dpkg
|
| seems to work as advertised.
correction... the link i made no longer has the setuid bit,
but neither has the newly installde binary.
# apt-get --reinstall install traceroute-nanog
Reading Package Lists... Done
Building Dependency Tree... Done
0 upgraded, 0 newly installed, 1 reinstalled, 0 to remove and 32 not upgraded.
Need to get 0B/33.2kB of archives.
After unpacking 0B of additional disk space will be used.
Do you want to continue? [Y/n]
(Reading database ... 35179 files and directories currently installed.)
Preparing to replace traceroute-nanog 6.3.9-3 (using .../traceroute-nanog_6.3.9-3_i386.deb) ...
Unpacking replacement traceroute-nanog ...
Setting up traceroute-nanog (6.3.9-3) ...
# ls -l /usr/sbin/traceroute-nanog
-rwxr-xr-x 1 root root 24408 Dec 5 19:51 /usr/sbin/traceroute-nanog
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Scott James Remnant <scott@netsplit.com>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #68 received at 225692@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
On Tue, Jan 20, 2004 at 03:41:12PM +0200, Erno Kuusela wrote:
> On Sun, 18 Jan 2004, Erno Kuusela wrote:
> | | Available now at people.debian.org/~jfs/dpkg
> |
> | seems to work as advertised.
>
> correction... the link i made no longer has the setuid bit,
> but neither has the newly installde binary.
>
> # apt-get --reinstall install traceroute-nanog
>
traceroute-nanog doesn't contain a setuid binary...
-rwxr-xr-x root/root 24408 2003-12-05 17:51:19 ./usr/sbin/traceroute-nanog
Tested with at instead:
descent:~# ls -l /usr/bin/at
-rwsr-xr-x 1 root root 34488 Jan 18 2002 /usr/bin/at
descent:~# ln /usr/bin/at foo
descent:~# ls -l foo
-rwsr-xr-x 2 root root 34488 Jan 18 2002 foo
descent:~# dpkg -i at_3.1.8-11_i386.deb
(Reading database ... 11564 files and directories currently installed.)
Preparing to replace at 3.1.8-11 (using at_3.1.8-11_i386.deb) ...
Stopping deferred execution scheduler: atd.
Stopping deferred execution scheduler: atd.
Unpacking replacement at ...
Setting up at (3.1.8-11) ...
Starting deferred execution scheduler: atd.
descent:~# ls -l /usr/bin/at
-rwsr-xr-x 1 root root 34488 Jan 18 2002 /usr/bin/at
descent:~# ls -l foo
-rw------- 1 root root 34488 Jan 18 2002 foo
Scott
--
Have you ever, ever felt like this?
Had strange things happen? Are you going round the twist?
[signature.asc (application/pgp-signature, inline)]
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Scott James Remnant <scott@netsplit.com>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #73 received at 225692@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
Here's a patch that just copies the code from main/remove.c which is
known to work.
Scott
--
Have you ever, ever felt like this?
Had strange things happen? Are you going round the twist?
[dpkg-3-stashed-hardlink.patch (text/plain, attachment)]
Tags added: fixed
Request was from Scott James Remnant <scott@netsplit.com>
to control@bugs.debian.org.
(full text, mbox, link).
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Javier Fernández-Sanguino Peña <jfs@computer.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #80 received at 225692@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
Hi, I'm just curious because reviewing this bug it looks like the patch
provided by Scoot is _precisely_ the same patch I provided (albeit with the
comments describing the behaviour removed). As I mentioned in the bug
report, the code I provided was taken verbatim from remove.c (well, it has
been reindented but it's exaclty the same code).
Reviewing the NMU I can see that the applied patch does not include any of
the comments I provided (nor references) which, I believe, gave appropiate
credit. After all, Brian Hatch found this issue first (even if he did not
report it himself).
Is there any reason why my comments (and the debug line I added) have been
removed from the patch?
Regards
Javier
PS: Giving credit would have been also a nice thing, BTW.
PPS: I would have liked to hear dpkg-dev's team input in this issue,
changes in this package are of significant importance that I did not want
to risk doing an NMU.
PPS: CCing me in the replies to Erno would have been nice too.
[signature.asc (application/pgp-signature, inline)]
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Scott James Remnant <scott@netsplit.com>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #85 received at 225692@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
On Fri, 2004-02-27 at 22:19, Javier Fernández-Sanguino Peña wrote:
> Reviewing the NMU I can see that the applied patch does not include any of
> the comments I provided (nor references) which, I believe, gave appropiate
> credit. After all, Brian Hatch found this issue first (even if he did not
> report it himself).
>
> Is there any reason why my comments (and the debug line I added) have been
> removed from the patch?
>
A ~60 line comment in the middle of a ~1,000 line function isn't really
a great place to put that kind of thing. What the code does is fairly
obvious, and why it does it is documented in the ChangeLog entry for the
patch.
I dislike, in general, long-winded discussions of code in the middle of
functions. Various articles and papers are interesting to the security
expert, but not to the maintainer of the code.
The *right* place for such discussions, and provision of relevant papers
and articles is the bug tracking system. The change to the code is then
accompanied by a reference to the bug number (in debian/changelog) for
the interested reader.
So whilst the full history of a bug/change, who first noticed it,
suggested it and/or found it, etc. etc. are all interesting -- the BTS
is what keeps that information, not comments above every single block of
code in the source.
Scott
--
Have you ever, ever felt like this?
Had strange things happen? Are you going round the twist?
[signature.asc (application/pgp-signature, inline)]
Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Development <debian-dpkg@lists.debian.org>:
Bug#225692; Package dpkg.
(full text, mbox, link).
Acknowledgement sent to Javier Fernández-Sanguino Peña <jfs@computer.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Development <debian-dpkg@lists.debian.org>.
(full text, mbox, link).
Message #90 received at 225692@bugs.debian.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
On Fri, Feb 27, 2004 at 11:02:08PM +0000, Scott James Remnant wrote:
> > Is there any reason why my comments (and the debug line I added) have been
> > removed from the patch?
> >
> A ~60 line comment in the middle of a ~1,000 line function isn't really
> a great place to put that kind of thing. What the code does is fairly
> obvious, and why it does it is documented in the ChangeLog entry for the
> patch.
Ok. But you didn't move my comments to the Changelog entry either.
> I dislike, in general, long-winded discussions of code in the middle of
> functions. Various articles and papers are interesting to the security
> expert, but not to the maintainer of the code.
Of course, you are entitled to your own opinion. In any case, you dropped
what I believe is a good source of information on _why_ was the change
introduced.
> The *right* place for such discussions, and provision of relevant papers
> and articles is the bug tracking system. The change to the code is then
> accompanied by a reference to the bug number (in debian/changelog) for
> the interested reader.
I disagree. Software developers do not necessarily need to go to the BTS,
they use the documentation provided in the package or the source code
itself.
> So whilst the full history of a bug/change, who first noticed it,
> suggested it and/or found it, etc. etc. are all interesting -- the BTS
> is what keeps that information, not comments above every single block of
> code in the source.
It's important enough to give credit where credit is due, and you have
neglected that. Giving credit (to Brian Hatch for example) encourages
positive feedback. If you disliked the comment in the source code, you
could have moved it to the Changelog entry, if you did not find that was
the place either you could have updated dpkg's documentation that describes
_how_ it is supposed to behave.
Even if it was a fix for a security issue, It's worth documenting it
further, "stashing away" is not a good enough description. As far as I see
from your NMU this was the only bug which was a significant change in
dpkg's behaviour, I don't believe any other programs rely on this, but
documenting more why it's being introduced would be of benefit both to
casual observers of the Changelog and to other package manager's software
developers.
Just my 2c.
Regards
Javier
[signature.asc (application/pgp-signature, inline)]
Reply sent to Scott James Remnant <scott@netsplit.com>:
You have taken responsibility.
(full text, mbox, link).
Notification sent to Erno Kuusela <erno-debbugs@erno.iki.fi>:
Bug acknowledged by developer.
(full text, mbox, link).
Message #95 received at 225692-close@bugs.debian.org (full text, mbox, reply):
Source: dpkg
Source-Version: 1.10.19
We believe that the bug you reported is fixed in the latest version of
dpkg, which is due to be installed in the Debian FTP archive:
dpkg-dev_1.10.19_all.deb
to pool/main/d/dpkg/dpkg-dev_1.10.19_all.deb
dpkg-doc_1.10.19_all.deb
to pool/main/d/dpkg/dpkg-doc_1.10.19_all.deb
dpkg_1.10.19.dsc
to pool/main/d/dpkg/dpkg_1.10.19.dsc
dpkg_1.10.19.tar.gz
to pool/main/d/dpkg/dpkg_1.10.19.tar.gz
dpkg_1.10.19_i386.deb
to pool/main/d/dpkg/dpkg_1.10.19_i386.deb
dselect_1.10.19_i386.deb
to pool/main/d/dpkg/dselect_1.10.19_i386.deb
A summary of the changes between this version and the previous one is
attached.
Thank you for reporting the bug, which will now be closed. If you
have further comments please address them to 225692@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.
Debian distribution maintenance software
pp.
Scott James Remnant <scott@netsplit.com> (supplier of updated dpkg package)
(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Format: 1.7
Date: Mon, 8 Mar 2004 19:05:32 +0000
Source: dpkg
Binary: dpkg-doc dpkg dselect dpkg-dev dpkg-static
Architecture: source all i386
Version: 1.10.19
Distribution: unstable
Urgency: high
Maintainer: Dpkg Development <debian-dpkg@lists.debian.org>
Changed-By: Scott James Remnant <scott@netsplit.com>
Description:
dpkg - Package maintenance system for Debian
dpkg-dev - Package building tools for Debian
dpkg-doc - Dpkg Internals Documentation
dselect - a user tool to manage Debian packages
Closes: 139781 157437 168443 170953 190611 199489 199693 211566 213038 213543 213846 217286 217943 221989 222760 225692 228253 228379 232025 232916 235266
Changes:
dpkg (1.10.19) unstable; urgency=high
.
* Distinguish unmet build dependencies from build conflicts.
Closes: #217943, #235266.
* Force NULL-termination of all tar file entry names. Closes: #232025.
* Allow dselect to use the full window width. Closes: #139781.
* Pass correct number of arguments for format string when out of disk
space. Closes: #213038, #217286, #213543, #213846.
* Remove duplicated entries from ChangeLog. Closes: #157437.
* Fix dpkg-buildpackage when used with PGP. Closes: #232916.
* Update support for Debian FreeBSD. Closes: #211566.
* Store Architecture in the status file. Closes: #228253.
* Don't print offending lines in md5sum. Closes: #170953.
* Check bounds of md5sum lines. Closes: #168443, #199489, #199693.
.
dpkg (1.10.18.1) unstable; urgency=medium
.
* Non-maintainer upload to fix release-critical bugs.
* Terminate string buffer in main/remove.c. Closes: #228379.
* Prevent stashing of hardlinked devices and setuid or setgid binaries
by removing permissions on upgrade as well as on remove.
Closes: #225692.
* Update dpkg conflicts to << 1.10, instead of 1.9.
Closes: #190611, #221989, #222760.
Files:
5a4c39cb6903694ec7ff0ebcd5afc33d 798 base required dpkg_1.10.19.dsc
a735a1f14cc985ad083b46bce425001b 1547265 base required dpkg_1.10.19.tar.gz
86386707c685a60c4132def2494b3657 1086080 base required dpkg_1.10.19_i386.deb
5bf8ab50684b58b8619f2c7d982ac47f 95024 base required dselect_1.10.19_i386.deb
088fe395e33835351e2ccdc3f8122a31 114618 utils standard dpkg-dev_1.10.19_all.deb
294b7c2bf86172671a91b050c6db8a1a 10636 doc optional dpkg-doc_1.10.19_all.deb
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
iD8DBQFATMchIexP3IStZ2wRAoQRAJ0Q5CxELST85r5oNEY3nnZE6TB/mgCggxyX
0sfA6HaAPzloTBPKab+L3nw=
=R85a
-----END PGP SIGNATURE-----
Send a report that this bug log contains spam.
Debian bug tracking system administrator <owner@bugs.debian.org>.
Last modified:
Fri Aug 2 03:51:44 2024;
Machine Name:
buxtehude
Debian Bug tracking system
Debbugs is free software and licensed under the terms of the GNU
Public License version 2. The current version can be obtained
from https://bugs.debian.org/debbugs-source/.
Copyright © 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson,
2005-2017 Don Armstrong, and many other contributors.