Fragen? Antworten!
assert(len+100 > len);len ist ein int. Hier sind zwei Beispielprogramme: int.c (signed) und unsignedint.c (unsigned). Nicht zu fassen. Das betrifft konkret den gnupg-Patch. FINSTER. Probiert das mal bei euch aus. Ich habe hier amd64 und x86 und kann das mit gcc 4.1.1 bei beiden nachvollziehen. Sehr gruselig. Was jetzt? Ich werde mal einen Bug bei gcc einreichen, aber ich fürchte ja, daß da wieder irgendein Language Lawyer kommt und mir sagt, daß man das formaljuristisch so auslegen kann in C.
Update: Hier ist der Bug, und wie ihr sehen könnt, findet der gcc-Typ, der an dem Problem Schuld ist (das gibt er in dem anderen Code-Stück zu), daß das in Ordnung ist, wenn Leute geownt werden, weil er eine Optimierung formaljuristisch rechtfertigen kann. Krank. Da fragt man sich ja schon, für was man eigentlich Audits macht, wenn man dann so torpediert wird. Oh, übrigens: mit icc (dem Intel-Compiler) kriegt man korrekt eine Assertion.
Update: Der Kracher: das macht so viel Software kaputt, daß die autoconf-Leute darüber nachdenken, -fwrapv zu den Default-Optimierungen zu tun, was das alte Verhalten wieder anschalten. Way to go, GNU! Macht ja nichts, wenn euer Schweinecompiler anderer Leute Software kaputt macht, solange ihr einen Workaround für eure eigene Software habt!1!!
Update 2: Offenbar optimieren auch gewisse gcc 2.95.* und gcc 3 Versionen (3.4.6 auf MirBSD und 3.4.3 auf Dragonfly BSD) diesen Code weg, aber der offizielle Workaround (-fwrapv) existiert nicht bei diesen gcc-Version. Die Antwort des gcc-Typen, der an dem ganzen Problem Schuld ist (Andrew Pinski) ist: gcc 2 und 3 sind nicht mehr supported und im Übrigen "why should we support older GCC when we can barrely support the current ones".
Update 3: Und Florian Weimer hat schon 2002 gesagt, daß das mal passieren würde. (Danke, Sebastian)
Bitte beachtet:
Update: Und schon hat jemand (Fabian Keil, danke!) einen Fuckup gefunden. Neues Diff (Jan 15 17:20) geuploaded.
The man who does not read good books has no advantage over the man who cannot read them.Klar könnt ihr euch jetzt hinsetzen, denken, der Fefe hat geguckt, der wird da schon alles Wichtige gefunden haben. Aber ihr seid dann genau so weit wie vorher, ihr vertraut immer noch anderer Leute Urteil, und gelernt habt ihr auch nichts dabei. Lernt mal Code lesen, und lest dann auch Code. Das ist der wichtigste Schritt zur Emanzipierung und Aufklärung im Digitalzeitalter. Das wird alles immer nur noch komplizierter! Je länger ihr wartet, desto größer die Hürden.
Was tut man in solchen Fällen? Man spricht den Autor an. Habe ich gemacht. Erst mal als Testballon eine Frage zu seinem ASN.1-Code, den Funktionsnamen ins Subject.
Schreibt er mir zurück, ich müsse schon sagen, wo ich diese Codefragmente genau gefunden hätte. Ich schreib also zurück, äh, Funktionsname == Subject. Gut, übersehen, danke ich mir, kann schon mal vorkommen. Was kommt zurück?
Wenn Du nicht in der Lage bist halbwegs anständige Informationen zu liefern, kann ich Dich wohl kaum unterstützen.Diese komplette Fehleinschätzung der Dinge ist mir bisher nur einmal unter gekommen: bei Jörg Schilling. Leute, wenn ich in eurem Code Bugs finde und euch das mitteile, dann bin ICH der Unterstützende, und zwar unterstütze ich euch. Nicht umgekehrt. Wer da der Illusion erliegt, daß er als Autor mir Support zukommen läßt, der vertreibt seine einzige Chance auf kostenloses Bugfinden, und — er hat es auch nicht anders verdient. Aber wartet, geht noch weiter.
Was glaubst Du wohl, wie oft ich so eine Funktion hier rumfliegen habe.Ich habe geguckt. Einmal in gnupg 1.4.6 und einmal in gnupg 2.0.1, und zwar der identische Code, cut&paste. Soviel dazu, Werner. Was soll ich sagen, auf solche Spirenzchen habe ich überhaupt gar keinen Bock. Dann kann gnupg halt von mir aus schlecht bleiben. Ich habe mal ein paar Vorträge auf Sicherheitskonferenzen angemeldet, "Auditing GnuPG", da werde ich dann halt die Details erzählen, und demnächst taucht dann hier ein Patch auf, den könnt ihr dann alle haben. Das war jedenfalls mit Sicherheit das letzte Mal, daß ich ihm zu helfen versucht habe.
Im Übrigen fände ich das alles nur halb so erbärmlich, wenn Werner nicht öffentliche Förderkohle abgegriffen hätte, um seine Softwareentwicklung zu finanzieren. Damit ist er jetzt bei mir persönlich auf einer Ansehensstufe mit Steuerhinterziehern.
2002-03-12 Werner Koch <wk@gnupg.org&bt;Seit dem gab es da noch andere Probleme, allerdings sieht der Code völlig anders aus in gnupg. Das scheint tatsächlich von zlib 1.1.4 zu sein. GRUSEL!!!Merged changes from zlib 1.1.4.
Das wird offenbar nur reingelinkt, wenn es im System keine zlib gibt. Noch mal Schwein gehabt.
if ( !initialized ) {
volatile ulong aa, bb; /* we really want the uninitialized value */
ulong a, b; initialized = 1;
/* also this marker is guessable it is not easy to use this
* for a faked control packet because an attacker does not
* have enough control about the time the verification does
* take place. Of course, we can add just more random but
* than we need the random generator even for verification
* tasks - which does not make sense. */
a = aa ^ (ulong)getpid();
b = bb ^ (ulong)time(NULL);
memcpy( marker, &a, SIZEOF_UNSIGNED_LONG );
memcpy( marker+SIZEOF_UNSIGNED_LONG, &b, SIZEOF_UNSIGNED_LONG );
}
Ich habe da genau so viel Kontext wie ihr, aber ich würde das so interpretieren, daß Herr Koch hier ein Protokoll mit in-band signalling gebaut hat, und ein Angreifer hier Pakete einfügen kann, und er will das jetzt "reparieren", indem er "Zufallswerte" einfügt, die der Angreifer nicht raten können soll.Also ich könnte euch jetzt erzählen, daß Werte auf dem Stack alles andere als zufällig sind, daß PID und Uhrzeit auch eher gut ratbar sind, aber die eigentliche Nachricht ist: gnupg hat einen "control channel", der in-band signalling benutzt, und auf dem Angreifer Daten einschleusen können. Mehr muß man nicht sagen. Und diesen Müll-Code habe ich seit Jahren eingesetzt! OMFG! Ich glaube nicht, daß das mit einem Patch fixbar ist. Wegschmeißen, neu machen. Im Übrigen schreibt man das "although", nicht "also".
if (*p == '*')Sieht ja an sich OK aus, außer man beschäftigt sich mal mit der Zahlendarstellung in Computern. Im Zweierkomplement gibt es eine negative Zahl (0x80000000 auf 32-bit Systemen), für die es keine Positiv-Darstellung gibt. Wenn man diese Zahl negiert, kommt sie wieder selbst raus. Für abs() bedeutet das:
{
++p;
total_width += abs (va_arg (ap, int));
}
abs(0x80000000) == 0x80000000 == -2147483648Für diesen Code heißt das, daß die offensichtlich unterliegende Annahme falsch ist, daß dieser Code immer nur Zahlen drauf addiert. Insbesondere kann man so den Code dazu kriegen, weniger Platz zu allozieren, als dann am Ende reingeschrieben wird.
Was mich ja besonders irritiert: dieser Code kommt von libiberty, das ist Teil von gcc/binutils/gdb. Das ist noch keinem aufgefallen?
Und als ich gerade dabei war, dachte ich mir, mhh, printf liefert die Anzahl der Zeichen zurück, die er hätte schreiben wollen. Als int. Was, wenn das so groß wird, daß es negativ wird? Sollte printf das abfangen? -1 zurück geben? Mal gucken, was die glibc macht:
#include <stdio.h>Wenn man das Programm aufruft, frißt er ein Gig RAM, läuft 17 Sekunden (auf meinem fetten Athlon 64), und schreibt dann -2147483647.int main() {
printf("%d\n",snprintf(0,0,"%*d %*d",0x40000000,1,0x40000000,1));
}
Ich hab das jetzt für die dietlibc so gemacht, daß *printf dann -1 zurück gibt. Offenbar ist das unter Solaris jetzt schon so, und wenn ich nicht der erste bin, der das so macht, dann trifft mich auch keine Schuld wegen der Kompatibilitätshölle :-)
Ich habe auch gerade eine Mail vom mirBSD-Projekt gekriegt, da hat jemand den Eintrag hier gesehen und nen Patch eingespielt, die liefern jetzt auch -1 zurück. Das war fix, keine zwei Stunden Turnaround!
Was mich daran am meisten schockiert: das hat keiner gemerkt. Niemand. Avahi gibt es nen Jahr? Länger? Und niemand außer mir hat offenbar mal unabhängig mdns implementiert. Der Effekt ist, daß wenn man mit dem dietlibc Resolver nach einem von avahi beantworteten Host fragt, die Antwort gefressen wird. Ich habe mal einen Workaround in die dietlibc eingebaut, aber hey, ist das zum Kotzen oder was?
Update: Jemand hat dem avahi-Autor Bescheid gesagt, und der hat mich jetzt per Email angepinkelt. Die Situation stellt sich tatsächlich etwas differenzierter dar als bisher dargestellt. avahi implementiert die aktuelle Spec, ich eine ältere. Meine Schuld, hätte ich vor dem Meckern mal gucken können. In der aktuellen Spec steht, daß das RD Bit Null sein soll beim Senden, und daß man es beim Reinkommen ignorieren muß. Das stand früher nicht in der Spec, aber man muß die Wayback Machine bemühen, um auf frühere Versionen zugreifen zu können; offenbar sind den zeroconf-Leuten die früheren Versionen peinlich. Anyway, ich habe mir avahi bei der Gelegenheit noch mal kurz angesehen, und muß sagen, ist nicht so schlimm wie gnupg, aber leider auch nicht so richtig toll. Ich hab da jetzt keinen kompletten Audit gemacht, aber schon mal direkt nen (nicht exploitbaren) off-by-one buffer overflow gefunden, und eine Endlosschleife in der DNS Dekompression. Ich denke mal, für avahi wird es demnächst ein Update geben. Der CVS-Code bei der dietlibc akzeptiert jetzt konform zur aktuellen Spec auch mDNS-Pakete, die das Bit anders setzen als es bei ihnen ankam. Ich möchte an dieser Stelle übrigens die mDNS-Specschreiber aufrufen, diesen Blödsinn zu lassen, und die Regeln von DNS bezüglich dieser Bits weiter bestehen zu lassen — Eingabe == Ausgabe. Dann kann man mehr Code wieder verwenden und ihr ganzes Gefasel von wegen Code-Wiederverwerten und Protokoll-Beibehalten stimmt dann auch tatsächlich. Eines ihrer Argumente ist ja gerade, daß man die APIs beibehalten kann, und daraus folgt, daß res_query das Paket nicht selber baut, sondern res_mkquery tut das, und daher ist es auch nicht kosher, wenn res_query dann im Paket rumdoktorn soll, um das RD Bit zu löschen (immerhin sagt die mDNS-Spec dafür nur SHOULD, daher spare ich mir das). Leider sagt die aktuelle mDNS-Spec auch, daß bei rausgehenden Antworten das RD-Bit Null sein soll, und die DNS-Spec sagt, daß bei Antworten dieses Bit übernommen werden soll. Wenn die das Gefasel mit API-Beibehalten also ernst nehmen, dann sollte hier ganz klar Eingabe == Ausgabe gelten.
Also ich mache sowas ja beruflich, und habe mir mal überlegt, ob ich nicht gnupg auditieren will. Aber das Ding ist ja so derartig am Explodieren von Release zu Release, das ist völlig unakzeptabel. gnupg ist einmal komplett Teil der TCB, solchen Code muß man minimieren. Was macht Herr Koch stattdessen? Baut da Grafik-Anzeigen rein, einen Agent, eine nicht abschaltbare Smartcard-Library, das (binäre!) Dateiformat ist zum Göbeln, der Footprint bei gnupg 2 beinhaltet auch noch libusb (WTF!?), pth (pthread-Abstraktion von GNU), hat LDAP drin (!?!?), ... hier ist mal ein Codezeilen-Vergleich:
while(fgets(line,MAX_LINE,input))Irgendwie schon schade, daß es dieses Jahr kein Codequartett auf dem Congress gibt. MAX_LINE ist so definiert:
[...]
if(strlen(line)+keylen>keymax)
{
keymax+=200;
tmp=realloc(key,keymax+1);
[...]
key=tmp;
}
strcpy(&key[keylen],line);
keylen+=strlen(line);
#define MAX_LINE (6+1+1024+1)und keylen und keymax starten als 0. WTF?!