On Thu, Feb 13, 2003 at 06:55:54PM +0100, Josselin Mouette wrote:
> > (a) avoiding packages that've been trojaned upstream entering
> > Debian, either through a Debian maintainer or via the
> > sponsorship system?
> You cannot ask the maintainers to review every single line of upstream
> code,
Sure I can. It's easy: "hey, maintainers: would you mind reviewing every
line of upstream code in future? Thanks!" See?
> especially when it is moving fast (I don't know whether it is the
> case for micq). Or else, we will have to seriously decrease the number
> of packages we provide.
$ diff -rb micq-0.4.9.4/ micq-0.4.10.1/ | wc -l
52254
Skimming that diff picks up some oddities, eg:
] +++ micq-0.4.10.1/include/config.h.in 2003-01-25 05:03:12.000000000 +1000
] @@ -247,6 +271,9 @@
] first (like Motorola and SPARC, unlike Intel and VAX). */
] #undef WORDS_BIGENDIAN
]
] +/* "" */
] +#undef __Dbn__
] +
(Why doesn't that option have a description? The others seem to.)
] +++ micq-0.4.10.1/src/buildmark.c 2003-01-11 03:30:30.000000000 +1000
...
] +#ifndef EXTRAVERSION
] +#ifdef __AMIGA__
] +#define EXTRAVERSION "AmigaOS hand compiled"
] +#elif defined (__BEOS__)
] +#define EXTRAVERSION "BeOS hand compiled"
...
] +#elif __Dbn__ != -1
] +#define EXTRAVERSION "De" "bi" "an hand compiled"
(WTF? Why split "Debian"? That's nuts.)
Hrm, you know:
+++ micq-0.4.10.1/src/cmd_pkt_v8_snac.c 2003-01-22 08:04:42.000000000 +1000
+ if (type == 2)
+ {
+ cont->flags |= CONT_INTIMATE;
+ cont->flags &= CONT_HIDEFROM;
+ }
+ else if (type == 3)
+ {
+ cont->flags |= CONT_HIDEFROM;
+ cont->flags &= CONT_INTIMATE;
+ }
is completely irrelevant to the topic at hand, but looks amazingly stupid.
CONT_HIDEFROM appears to be 2, CONT_INTIMATE appears to be 4. So in both
cases, unless we're talking about DMA or something, the |= does nothing.
"flags &= ~CONT_foo" would make sense (to clear that bit), but what's
written is baloney. From other bits of code it looks like HIDEFROM and
INTIMATE are meant to be mutually exclusive, so it definitely seems like
there're missing ~'s.
Anyway.
Of course, the particular code we're looking for jumps right out at you,
especially when you know it's there.
I didn't see anything else obviously wrong. I didn't look at any of the
automake generated stuff (that *is* too much to ask), and I didn't get
any sort of feel for most of the i18n and UTF stuff. In an unprejudiced
skim, I think I would've found all the suspicious occurences of __Dbn__,
and any of them would've let me cotton on to the trickery. Skimming it
all seemed to take about an hour.
The code could definitely do with a bunch more named constants (5? wtf
is that?), and the "Session" to "Connection" conversion is definitely
a nuisance. But otherwise, it's not really that tricky.
In any event, the question isn't about how hard this is, it's about
what alternatives there are. Not packaging stuff that's already been
trojaned would not have avoided this problem; nor would pretending the
problem doesn't exist or can't happen to us. We've been exploited once --
happily in a manner that doesn't cause major problems. We've already seen
that this is a common attack -- a bunch of upstream sites and mirrors
have been cracked and had source code trojaned. We're vulnerable. What
can we do to fix this hole? What will we do to fix it?
> > (b) how to best interact with upstream maintainers that can get
> > exceedingly obnoxious?
> The author can be obnoxious without trojaning the code. This is a
> different matter.
Honestly, I find it hard to get really riled up about what was actually
in the code. It didn't exactly wreck people's systems, did it?
Cheers,
aj
--
Anthony Towns <aj@humbug.org.au> <http://azure.humbug.org.au/~aj/>
I don't speak for anyone save myself. GPG signed mail preferred.
``Dear Anthony Towns: [...] Congratulations --
you are now certified as a Red Hat Certified Engineer!''
Attachment:
pgpP9FnYeD9HD.pgp
Description: PGP signature