Alte Klausurfrage zu Netzwerkprogrammierung, Fehler finden

Hi,

in der Funktion (o_pp) sollen zwei Fehler vorhanden sein, einer hat mit einer Fehlerhaften Variablendeklaration zu tun und der andere soll ein Sicherheitsloch darstellen, das ganze hat mit Netzwerkprogrammierung zu tun, ich habe keinen Schimmer, hat jemand eine Idee?

typedef struct header 
{
   int cmd_or_stat;
   int sz;
} header;

typedef struct packet 
{
   header hd;
   char data[512];
} packet;

void o_pp(int sock, packet *pkt) 
{
    int fd;
    int n = -1;
    switch (pkt->hd.cmd_or_stat) 
    {
       case OP_OPEN:
       rcv(sock, pkt->hd.sz, pkt->data);
       pkt->data[pkt->hd.sz] = 0;
       fd = open(pkt->data, O_RDWR);
       if(fd > 0) pkt->hd.cmd_or_stat = SUCCESS;
       else pkt->hd.cmd_or_stat = FAIL;
       pkt->hd.sz = 0;
       break;

       case OP_READ:
       if (fd > 0) n = read(fd, pkt->data, 512);
       if (n < 0) 
       {
         pkt->hd.cmd_or_stat = FAIL;
         pkt->hd.sz = 0;
       }else 
       {
         pkt->hd.cmd_or_stat = SUCCESS;
         pkt->hd.sz = n;
       }
       break;
       case OP_CLOSE:
       /*der folgende Code ist irrelevant*/
    }
}

mfg

was ist das für eine function

rcv(sock, pkt->hd.sz, pkt->data);

Gemeint ist die Funktion rcv aus der Header:

#include <sys/socket.h>

also ich kannte da bisher nur die, und die braucht 4 parameter und der 2. ist der daten buffer

recv(int socket, void *buffer, size_t length, int flags);

Mhhh ja das stimmt dann scheint es vielleicht nicht so sehr um die Details zu gehen.

wenns nicht so um die details geht worum denn dann

du solltest dir mal überlegen das du die strings nicht null terminierst dadurch wär evtl. der ein oder andere angriff möglich.

gehört das recv wirklich in einen block mit open oder sollte es eher heißen

case OP_RECV:
  pkt->hd.sz = recv(sock, pkt->data ,511);
  if(recv>=0)
    pkt->data[pkt->hd.sz] = 0;
break;
case OP_OPEN:
  fd = open(pkt->data, O_RDWR);
  if(fd > 0) pkt->hd.cmd_or_stat = SUCCESS;
  else pkt->hd.cmd_or_stat = FAIL;
break;

case OP_RECV:
  pkt->hd.sz = recv(sock, pkt->data ,511);
  if(pkt->hd.sz>=0)
    pkt->data[pkt->hd.sz] = 0;
break;

werd für heut lieber mal pause machen bei den ganzen fehlern die sich da einschleichen

Hi,

ja das recv gehört wirklich in einen Block mit Open.
Der Fehler mit der Variablendeklaration stört mich am
meisten, da der ja eigentlich nicht so schwer zu finden sein kann.
Die Funktion wird so aufgerufen. Müsste aber für die Fragestellung
irrelevant sein.

while (rcv(sock, sizeof(header), &pkt)) 
{
    o_pp(sock, &pkt);
    snd(sock, sizeof(header) + pkt.hd.sz,&pkt);
}

also was mir evtl. noch auffällt ist der parameter length der recv function welcher den typ size_t hat. size_t ist was anderes als int, aber das fällt eigentlich kaum auf, da man eh so gut wie nie derart große buffer verwendet oder?

Ja das stimmt das könnte falsch sein. Müsste die Variable fd nicht irgendwo anders definiert werden z.B. global? Da wenn OPEN abgearbeitet wurde und die Funktion verlassen wird um im nächsten Fall in READ geprüft wird ob fd > 0 ohne das sie irgendwie initialisert wurde?

C_Beginner4 schrieb:

Müsste die Variable fd nicht irgendwo anders definiert werden z.B. global? Da wenn OPEN abgearbeitet wurde und die Funktion verlassen wird um im nächsten Fall in READ geprüft wird ob fd > 0 ohne das sie irgendwie initialisert wurde?

linux_c89

Könnte die Sicherheitslücke sein dass wenn in hd.sz ein Wert >=512 steht ein buffer overflow verursacht werden kann wenn "open" verlangt wird?
Also sowas

header.cmd_or_stat=OP_OPEN;
header.sz=600;

Dann würden beim recv-Aufruf 600 bytes in data geschrieben werden
Wenn das Ding als root läuft könnte auch ein übergeben von "../../../etc/passwd" (oder wieviele Ordner man auch immer hochmuss) problematisch sein

EOP

linux_c89 schrieb:

Wenn das Ding als root läuft könnte auch ein übergeben von "../../../etc/passwd" (oder wieviele Ordner man auch immer hochmuss) problematisch sein

Das sieht gut aus: Scheint klassisches LFI zu sein.

Braucht i.A. gar keine root-Rechte und ein ../../../../../../../../../../../etc/passwd findet/öffnet auch passwd. Du musst die exakte Anzahl von parent folders gar nicht wissen/angeben.

Edit #1:
Kann man ganz einfach mit

cd ../../../../../../../../../../../

testen. Funktioniert bei *NIX ebenso wie bei DOS/Win.

Edit #2:
File wird auch noch Read+Write geöffnet. Ist natürlich noch übler.

linux_c89

naja wenn das Programm keine rootrechte hat wird das Öffnen der Datei mangels Rechten fehlschlagen
Und du brauchst halt mindestens so oft .. wie du brauchst um nach / zu kommen
cd ../../../../../../../../../../../ funktioniert halt nicht wenn du dich in
/a/b/c/d/e/f/g/h/i/j/k/l/m befindest (wer aber sone Ordnerstruktur macht gehört bestraft^^)

EOP

linux_c89 schrieb:

naja wenn das Programm keine rootrechte hat wird das Öffnen der Datei mangels Rechten fehlschlagen

Übliche Rechte für /etc/passwd:

-rw-r--r--   1 root root    1908 Aug 26  2009 passwd

Lesen klappt nur bei

-rw-------   1 root root    1908 Aug 26  2009 passwd

nicht, bzw. dann nur als root.

linux_c89 schrieb:

Und du brauchst halt mindestens so oft .. wie du brauchst um nach / zu kommen
cd ../../../../../../../../../../../ funktioniert halt nicht wenn du dich in
/a/b/c/d/e/f/g/h/i/j/k/l/m befindest (wer aber sone Ordnerstruktur macht gehört bestraft^^)

Schon klar, war ja nur ein Beispiel.

linux_c89

Er öffnet die Datei aber als rw
Und selbst wenn nicht , ich würde es nicht als Sicherheitslücke bezeichnen wenn man die /etc/passwd lesen kann^^

Athar

Wozu überhaupt die indirekte Pfadangabe, man kann auch gleich /etc/passwd angeben.
Und als Sicherheitslücke würde ich das nicht unbedingt bezeichnen, das Zuverfügungstellen beliebiger Dateien scheint immerhin primärer Sinn und Zweck des Programms zu sein.

Dass das Programm mit den gewünschten Rechten läuft und diese Dienste nur dem gewünschten Nutzerkreis zur Verfügung stehen, ist Zuständigkeit des Betreibers.

linux_c89

Da hast du Recht ist mir gar nicht aufgefallen^^
Ich denke die garvierende Sicherheitslücke ist der buffer overflow wenn der (vom benutzer erzeugte!) header eine Größe von mehr als 512 ankündigt womit man den heap beliebig beschreiben kann