Vor ein paar Tagen gab es die Nachricht, daß u.a. Curl mit ChatGPT generierten Fehlerberichten zu gespammt wird, die nur völlig nutzlose Zeitverschwendung darstellt, weil die Tools Sachen falsch einschätzen. Das ist Exim jetzt auch passiert.

Bitte macht das nicht nach – Bughunting mit Tools

An sich ist es eine super Sache, daß sich Menschen um die Sicherheit Ihrer Produkte Gedanken machen und aktiv helfen wollen, bei Problemen einzugreifen. Leider machen einige Tools den Entwicklern von OpenSource Projekten das Leben schwer ( siehe Curl ).

Auch wenn im Pullrequest kein ChatGPT drin war, mußte sich jemand heute um so eine Zeitverschwendungsmeldung kümmern. In diesem Fall war das Ich und das Projekt um das es ging Exim:

• https://github.com/Exim/exim/pull/96

Damit alle Verstehen um was es geht:

Exim ist in C programmiert, eine Sprache die schon in den 70er benutzt wurde. Dem entsprechend sind die Standard-Funktionen extrem „schmal“ und ohne Rechenzeitverbrennung programmiert worden, meint: Geschwindigkeit war wichtiger als Sicherheit, weil es „noch keine Exploits gab“ und, hauptsächlich, die CPUs weniger leisten konnten.

strcpy() ist eine eingebaute C-Funktion die Inhalte von Speicher A in Speicher B kopiert, bis diese auf eine 0 trifft.  Die binäre Null also 0x00 gilt in C als Stringterminator, also das Ende eines Textstrings. (Das ging genau so lange gut, bis UTF kam , aber das ist eine andere Geschichte.) strcpy(ZIEL,QUELLE) kopiert also genau solange Bytes von der Quelle ins Ziel, bis ein 0x00 kommt, EGAL ob der Speicherbereich „ZIEL“ ausreichend groß ist, um die kopierte Datenanzahl aufzunehmen. Wenn er es nicht ist, kommt es zum BUFFER-OVERFLOW und damit zu potentiellen Remote-Code-Executions, aber meistens nur zum Programmabsturz.

strncpy() ist eine Funktion, die genau das gleiche macht wie strcpy() nur, daß fest vorgegeben wird, wie viel kopiert werden kann, was das überschreiben des Zielspeichers verhindert, außer man gibt da einfach quatsch als Länge an, dann natürlich nicht mehr 😉

Beim Pullrequest im Eximsource gings um

  if ((int)strlen(pw->pw_dir) + (int)strlen(filename) + 1 > sizeof(buffer))
    {
    printf("exim_lock: expanded file name %s%s is too long", pw->pw_dir,
      filename);
    exit(EXIT_FAILURE);
    }

  strcpy(buffer, pw->pw_dir);
  strcat(buffer, filename);
  filename = buffer;
}

. Das sollte so ersetzt werden:

– strcpy(buffer, pw->pw_dir);
+ strncpy(buffer, pw->pw_dir);

Leider braucht strncpy() 3 Argumente, nämlich auch die Größe des Zielspeichers:

char *strncpy(char dst[restrict .dsize], const char *restrict src, size_t dsize);

Wenn man da nur 2 Argumente liefert, schlägt das Kompilieren des Sources fehl, außer irgendeine Compiler Magie erkennt die Funktion und füllt den dritten Parameter heimlich auf. Sich darauf zu verlassen ist genauso fahrlässig, wie noch strcpy() zu benutzen, ohne vorher geprüft zu haben, ob das sicher klappen kann.

Die Eximdevs haben das aber bereits richtig gemacht:

  if ((int)strlen(pw->pw_dir) + (int)strlen(filename) + 1 > sizeof(buffer))
    {
    printf("exim_lock: expanded file name %s%s is too long", pw->pw_dir,
      filename);
    exit(EXIT_FAILURE);
    }

  strcpy(buffer, pw->pw_dir);
  strcat(buffer, filename);
  filename = buffer;
}

Der Test steht nur,Sourcecode bedingt, etwas weiter davon weg, weil man keine If-Than-Else Anweisung benutzt hat, sondern eine If-Not-Error Anweisung gemacht hat.  Es wird also der Fehlerfall abgefragt und dann ein Fehler ausgegeben,statt abzufragen ob das funktionieren kann, es dann zu tun und ANSONSTEN einen Fehler auszugeben.

Das ist völlig legitim, auch wenn ich persönlich das nicht so gemacht hätte, weil es dem Compiler erlaubt, etwas einfacheren Programmcode zu erzeugen.

Bitte nicht nachmachen

Wie Ihr oben selbst sehen könnt, ist alles ok gewesen. Irgendein Tool wird über strcpy() gestolpert sein und das routinemäßig der Person vor dem Monitor mitgeteilt haben. Die hatte aber keine Lust nachzusehen, ob überhaupt ein Problem vorliegt, oder keinen Plan davon, wie C funktioniert ( übrigens nicht meine Lieblingssprache 😉 ) und es dann einfach gemeldet.

Bitte macht das nicht nach! Das kostet uns als Entwickler einfach nur unnötige Zeit. Nehmt das Ergebnis und analysiert selbst nach, ob es überhaupt ein Problem gibt. Da haben alle was von, weil Ihr lernt was dazu, egal wie die Sache ausgeht, und ggf. habt Ihr auch ein Problem gefunden, das wirklich gelöst werden muß.

Beispiel:

Wenn ich …

char *ziel[500];
char *quellel[200];

memset(quelle,0,200);
strcpy(ziel,quelle);
return;

… habe, würde so ein Tool das strcpy() reklamieren, es könnte aber NIEMALS ein Problem darstellen. Wer als Mensch den Code sieht, versteht das auch, das Tool hat aber nur „grep ’strcpy(‚ sourcefile.c“ gemacht. Zum Problem wird so etwas hier:

function a(char* quelle) {

  char *ziel[500];
  strcpy(ziel,quelle);
  return;
}

Weil nichts über die Quelle, deren wahre Länge und Inhalt usw. bekannt ist. Hier muß man jetzt tatsächlich mit strncpy() arbeiten, um seinen Speicherbereich zu schützen:

function a(char* quelle) {

  char *ziel[500];
  strncpy(ziel,quelle,sizeof(ziel));
  return;
}

Und jetzt viel Spaß beim C lernen 😉