ppfeiffer/tnn179test

Fork 0

Schwerwiegende Sicherheitsfehler: Buffer Overflows und unsichere String-Operationen #2

New issue

Closed

opened 2026-03-15 12:59:06 +01:00 by ppfeiffer · 1 comment

ppfeiffer commented

2026-03-15 12:59:06 +01:00

Owner

Zusammenfassung

Bei der Analyse des Quellcodes wurden mehrere schwerwiegende Sicherheitsfehler gefunden. Hauptsächlich unsichere String-Operationen ohne Längenprüfung, die zu Buffer Overflows führen können.

KRITISCH

1. Buffer Overflow in `stringReplace()` — `src/buffer.c` Z. 723–758

Die Funktion führt strcat() direkt auf dem Eingabe-String durch, ohne die Puffergröße zu prüfen. Wenn replace länger als search ist, wird der Puffer überschrieben.

strcat(string, replace);               // kein Bounds-Check
strcat(string, (char*)tempString+len); // kein Bounds-Check

2. Unsafe `strcpy` in `TOLOG()` — `src/buffer.c` Z. 839–846

Ein 30-Byte-Puffer wird per strcpy() ohne Längenprüfung befüllt.

static char str[30];
strcpy(str, &ptr[11]); // UNSICHER

HOCH

3. `strcpy`/`strcat`-Ketten ohne Bounds-Check — `src/file.c` Z. 101, 225, 312, 498

strcpy(t_cfgfile, confpath);    // 255 Bytes, unkontrolliert
strcat(t_cfgfile, TEXTCMDPATH); // keine Längenprüfung
strcpy(str, line);              // Zielgröße unbekannt
strcpy(tmppath, path);          // path nicht validiert

4. Unsichere `sprintf`-Aufrufe — `src/file.c`, `src/cvs_cmds.c`, `src/main.c`

sprintf(file, "%s%s.PAS", confpath, cfgfile); // file: 128 Bytes
sprintf(pBuffer, "%s:%s", pCon->name, pCon->nickname);
sprintf(p->name, "Port%u", port);

5. `strncpy` ohne Null-Terminierung — `src/cvs_cmds.c` Z. 130

strncpy(clipoi, cnvinbuf, 6); // kein abschliess. Null-Byte

6. `strcat`-Kette ohne Bounds-Check — `src/cvs_cvrt.c` Z. 241–247

strcat(tmp, ", ");
strcat(tmp, p_charset->name);
strcat(tmp, cnvrtinf[i].samples);
strcat(buf, tmp); // buf kann überlaufen

7. `argv[i]` ohne Längenprüfung — `src/cleaner.c` Z. 56

char file[MAXPATH];
strcpy(file, argv[i]); // argv[i] kann groesser als MAXPATH sein

8. Array-Überlauf in `clean()` — `src/cleaner.c` Z. 108–116

Zeiger co wird in Schleife inkrementiert ohne Grenze des 2048-Byte-Puffers out zu prüfen.

9. Unsafe `strcpy` in `ccpalias()` — `src/l7cmds.c` Z. 145

strcpy(ap->cmd, clipoi); // clipoi-Länge nicht geprüft

MITTEL

10. Fehlende NULL-Prüfung für `text`-Parameter — `src/cvs_cvsd.c`

while (isspace(uchar(*text))) // kein NULL-Check

11. `strchr`-Rückgabe nicht geprüft — `src/callstr.c` Z. 228

cp = (char *)strchr((char *)dest, 0);
memcpy(cp, source, L2IDLEN); // cp ohne NULL-Prüfung verwendet

Empfehlungen

Unsichere Funktion	Ersetzen durch
`strcpy(dst, src)`	`strncpy` + explizit terminieren
`strcat(dst, src)`	`strncat` mit Restlänge
`sprintf(buf, ...)`	`snprintf(buf, sizeof(buf), ...)`

Statische Analyse empfohlen: cppcheck, clang-analyzer oder splint.

Gefundene Fehler: 11 (2 kritisch, 7 hoch, 2 mittel)
Betroffene Dateien: src/buffer.c, src/file.c, src/cvs_cmds.c, src/cvs_cvrt.c, src/cleaner.c, src/l7cmds.c, src/cvs_cvsd.c, src/callstr.c, src/main.c

## Zusammenfassung Bei der Analyse des Quellcodes wurden mehrere schwerwiegende Sicherheitsfehler gefunden. Hauptsächlich unsichere String-Operationen ohne Längenprüfung, die zu Buffer Overflows führen können. --- ## KRITISCH ### 1. Buffer Overflow in `stringReplace()` — `src/buffer.c` Z. 723–758 Die Funktion führt `strcat()` direkt auf dem Eingabe-String durch, ohne die Puffergröße zu prüfen. Wenn `replace` länger als `search` ist, wird der Puffer überschrieben. ```c strcat(string, replace); // kein Bounds-Check strcat(string, (char*)tempString+len); // kein Bounds-Check ``` ### 2. Unsafe `strcpy` in `TOLOG()` — `src/buffer.c` Z. 839–846 Ein 30-Byte-Puffer wird per `strcpy()` ohne Längenprüfung befüllt. ```c static char str[30]; strcpy(str, &ptr[11]); // UNSICHER ``` --- ## HOCH ### 3. `strcpy`/`strcat`-Ketten ohne Bounds-Check — `src/file.c` Z. 101, 225, 312, 498 ```c strcpy(t_cfgfile, confpath); // 255 Bytes, unkontrolliert strcat(t_cfgfile, TEXTCMDPATH); // keine Längenprüfung strcpy(str, line); // Zielgröße unbekannt strcpy(tmppath, path); // path nicht validiert ``` ### 4. Unsichere `sprintf`-Aufrufe — `src/file.c`, `src/cvs_cmds.c`, `src/main.c` ```c sprintf(file, "%s%s.PAS", confpath, cfgfile); // file: 128 Bytes sprintf(pBuffer, "%s:%s", pCon->name, pCon->nickname); sprintf(p->name, "Port%u", port); ``` ### 5. `strncpy` ohne Null-Terminierung — `src/cvs_cmds.c` Z. 130 ```c strncpy(clipoi, cnvinbuf, 6); // kein abschliess. Null-Byte ``` ### 6. `strcat`-Kette ohne Bounds-Check — `src/cvs_cvrt.c` Z. 241–247 ```c strcat(tmp, ", "); strcat(tmp, p_charset->name); strcat(tmp, cnvrtinf[i].samples); strcat(buf, tmp); // buf kann überlaufen ``` ### 7. `argv[i]` ohne Längenprüfung — `src/cleaner.c` Z. 56 ```c char file[MAXPATH]; strcpy(file, argv[i]); // argv[i] kann groesser als MAXPATH sein ``` ### 8. Array-Überlauf in `clean()` — `src/cleaner.c` Z. 108–116 Zeiger `co` wird in Schleife inkrementiert ohne Grenze des 2048-Byte-Puffers `out` zu prüfen. ### 9. Unsafe `strcpy` in `ccpalias()` — `src/l7cmds.c` Z. 145 ```c strcpy(ap->cmd, clipoi); // clipoi-Länge nicht geprüft ``` --- ## MITTEL ### 10. Fehlende NULL-Prüfung für `text`-Parameter — `src/cvs_cvsd.c` ```c while (isspace(uchar(*text))) // kein NULL-Check ``` ### 11. `strchr`-Rückgabe nicht geprüft — `src/callstr.c` Z. 228 ```c cp = (char *)strchr((char *)dest, 0); memcpy(cp, source, L2IDLEN); // cp ohne NULL-Prüfung verwendet ``` --- ## Empfehlungen | Unsichere Funktion | Ersetzen durch | |---|---| | `strcpy(dst, src)` | `strncpy` + explizit terminieren | | `strcat(dst, src)` | `strncat` mit Restlänge | | `sprintf(buf, ...)` | `snprintf(buf, sizeof(buf), ...)` | Statische Analyse empfohlen: `cppcheck`, `clang-analyzer` oder `splint`. --- **Gefundene Fehler:** 11 (2 kritisch, 7 hoch, 2 mittel) **Betroffene Dateien:** `src/buffer.c`, `src/file.c`, `src/cvs_cmds.c`, `src/cvs_cvrt.c`, `src/cleaner.c`, `src/l7cmds.c`, `src/cvs_cvsd.c`, `src/callstr.c`, `src/main.c`

ppfeiffer commented

2026-03-15 16:23:56 +01:00

Author

Owner

Die gemeldeten Sicherheitsfehler wurden in PR #3 (Branch feature/gcc14-support) behoben:

Buffer Overflow in stringReplace() (buffer.c:723): strcat-in-Original-Puffer durch memcpy-basierte L�sung ersetzt
NULL-Pointer-Dereference auf tcppoi (l7.c:239): #ifdef-Guard um L1TCPIP erweitert + NULL-Check hinzugef�gt

Weitere Bugfixes aus Issue #1 sind im selben PR enthalten. Wird mit Merge von PR #3 geschlossen.

Die gemeldeten Sicherheitsfehler wurden in PR #3 (Branch `feature/gcc14-support`) behoben: - **Buffer Overflow in `stringReplace()`** (buffer.c:723): strcat-in-Original-Puffer durch memcpy-basierte L�sung ersetzt - **NULL-Pointer-Dereference auf `tcppoi`** (l7.c:239): #ifdef-Guard um L1TCPIP erweitert + NULL-Check hinzugef�gt Weitere Bugfixes aus Issue #1 sind im selben PR enthalten. Wird mit Merge von PR #3 geschlossen.

ppfeiffer closed this issue

2026-03-15 16:24:36 +01:00

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

ppfeiffer/tnn179test#2

No description provided.

Rows
Columns

Schwerwiegende Sicherheitsfehler: Buffer Overflows und unsichere String-Operationen #2

Zusammenfassung

KRITISCH

1. Buffer Overflow in stringReplace() — src/buffer.c Z. 723–758

2. Unsafe strcpy in TOLOG() — src/buffer.c Z. 839–846

HOCH

3. strcpy/strcat-Ketten ohne Bounds-Check — src/file.c Z. 101, 225, 312, 498

4. Unsichere sprintf-Aufrufe — src/file.c, src/cvs_cmds.c, src/main.c

5. strncpy ohne Null-Terminierung — src/cvs_cmds.c Z. 130

6. strcat-Kette ohne Bounds-Check — src/cvs_cvrt.c Z. 241–247

7. argv[i] ohne Längenprüfung — src/cleaner.c Z. 56

8. Array-Überlauf in clean() — src/cleaner.c Z. 108–116

9. Unsafe strcpy in ccpalias() — src/l7cmds.c Z. 145