Git ist toll...

Wednesday, January 23. 2008

...als Beispiel für schlechten Code (das hier ist aus local-fetch.c):

static const char *path;
/* ... */
char filename[PATH_MAX];
strcpy(filename, path);
strcat(filename, "/objects/pack/pack-");
strcat(filename, sha1_to_hex(sha1));
strcat(filename, ".idx");
/* ... */
int main(int argc, const char **argv)
{
/* ... */
path = argv[arg];

Warum muss ich bloß ca. 5 Sekunden grep(1)en, um ein ideales Codebeispiel zu finden, wie man mit Strings, Pfaden und User Input nicht umgeht in C?

Update: weil gefragt wurde, wie's richtig geht, hier noch die Auflösung:

char filename[PATH_MAX];
snprintf(filename, sizeof(filename), "%s/objects/pack/pack-%s.idx", path, sha1_to_hex(sha1));

Das ist nicht nur sicherer, sondern auch kompakter und lesbarer. Nein, ich werde keinen Patch submitten.

Posted by Andreas Krennmair at 14:25 | Comments (5) | Trackbacks (0)

Defined tags for this entry: fun, git, rant, vcs

Related entries by tags:

Trackbacks

Trackback specific URI for this entry

No Trackbacks

Comments

Display comments as (Linear | Threaded)

Ich nehme an es geht dir um die fehlenden Länge-Überprüfungen. (Ist bei C ja angeblich immer eine sichere Vermutung). Was mich als C (Library) Unkundigen jetzt interessieren würde, wäre: wie schaut denn exakt die gleiche Funktion sicher|richtig gemacht aus? Vielleicht hast du ja mal etwas Zeit und Lust und zeigst hier ein korrigiertes Schnippsel...

Gruß & Danke Martin

#1 Martin on 2008-01-23 15:45 (Reply)

Danke!

#1.1 Martin on 2008-01-23 16:44 (Reply)

Das hat nur mindestens einen Nachteil: snprintf ist nicht portabel, da es nicht konsistent zwischen den verschiedenen Systemen implementiert ist. Wer weiss schon, ob am Ende noch ein NULL dranhaengt?

Nachzulesen wie meistens unter:
https://buildsecurityin.uscert.gov/daisy/bsi/838.html

(GIT-Quelltext nicht angesehen.)

#2 Karl Schulz on 2008-01-28 12:54 (Reply)

snprintf() ist in SuSv2 und C99 definiert, und unterscheidet sich im Verhalten nur in Sonderfällen des 1. bzw. 2. Parameters, die hier nicht zur Anwendung kommen. Wenn eine Plattform mit einer kaputten snprintf()-Implementierung daherkommt, so ist das nicht meine Angelegenheit. snprintf() nicht einzusetzen wäre wie wenn man strcpy() nicht einsetzen würde, nur weil es eine (hypothetische) Plattform gibt, bei der strcpy() von Strings länger als 256 Zeichen segfaultet.

#2.1 ak (Homepage) on 2008-01-28 22:21 (Reply)

>snprintf() nicht einzusetzen wäre wie wenn...
... als ob man sich vorher Gedanken macht. Es gibt bessere APIs fuer Strings (und Integerhandling und ...). djbs Variante ist doch auch brauchbar.

#2.1.1 Karl Schulz on 2008-01-29 11:08 (Reply)

Add Comment

Name
Email
Homepage
In reply to
Comment	Enclosing asterisks marks text as bold (word), underscore are made via _word_. Standard emoticons like :-) and ;-) are converted to images. You can use [geshi lang=lang_name [,ln={y\|n}]][/geshi] tags to embed source code snippets. To prevent automated Bots from commentspamming, please enter the string you see in the image below in the appropriate input box. Your comment will only be submitted if the strings match. Please ensure that your browser supports and accepts cookies, or your comment cannot be verified correctly. Enter the string from the spam-prevention image above:
	Remember Information? Subscribe to this entry