Dlaczego ten kod jest podatny na ataki polegające na przepełnieniu bufora?

int func(char* str)
{
   char buffer[100];
   unsigned short len = strlen(str);

   if(len >= 100)
   {
        return (-1);
   }

   strncpy(buffer,str,strlen(str));
   return 0;
}

Ten kod jest podatny na atak przepełnienia bufora i próbuję dowiedzieć się, dlaczego. Myślę, że ma to związek z lenotrzymaniem deklaracji shortzamiast an int, ale nie jestem pewien.

Jakieś pomysły?

W większości kompilatorów maksymalna wartość an unsigned shortto 65535.

Każda wartość powyżej jest zawijana, więc 65536 staje się 0, a 65600 staje się 65.

Oznacza to, że długie łańcuchy o odpowiedniej długości (np. 65600) przejdą kontrolę i przepełnią bufor.

Zastosowanie size_tdo przechowywania wynik strlen(), nie unsigned short, i porównać lendo wyrażenia, które bezpośrednio koduje rozmiar buffer. Na przykład:

char buffer[100];
size_t len = strlen(str);
if (len >= sizeof(buffer) / sizeof(buffer[0]))  return -1;
memcpy(buffer, str, len + 1);

Problem jest tutaj:

strncpy(buffer,str,strlen(str));
                   ^^^^^^^^^^^

Jeśli łańcuch jest większy niż długość bufora docelowego, strncpy nadal go kopiuje. Liczbę znaków łańcucha opierasz na liczbie do skopiowania, a nie na rozmiarze bufora. Prawidłowy sposób na zrobienie tego jest następujący:

strncpy(buffer,str, sizeof(buff) - 1);
buffer[sizeof(buff) - 1] = '\0';

To powoduje ograniczenie ilości danych kopiowanych do rzeczywistego rozmiaru bufora minus jeden dla znaku kończącego wartość null. Następnie ustawiamy ostatni bajt w buforze na znak null jako dodatkowe zabezpieczenie. Powodem tego jest to, że strncpy skopiuje do n bajtów, w tym kończący null, jeśli strlen (str) <len - 1. Jeśli nie, to wartość null nie jest kopiowana i masz scenariusz awarii, ponieważ teraz twój bufor ma niezakończony strunowy.

Mam nadzieję że to pomoże.

EDYCJA: Po dalszej analizie i wprowadzeniu informacji przez innych, możliwe kodowanie funkcji jest następujące:

int func (char *str)
  {
    char buffer[100];
    unsigned short size = sizeof(buffer);
    unsigned short len = strlen(str);

    if (len > size - 1) return(-1);
    memcpy(buffer, str, len + 1);
    buffer[size - 1] = '\0';
    return(0);
  }

Ponieważ znamy już długość ciągu, możemy użyć memcpy do skopiowania ciągu z lokalizacji, do której odwołuje się str, do bufora. Zauważ, że na stronie podręcznika dla strlen (3) (w systemie FreeBSD 9.3) podano, co następuje:

 The strlen() function returns the number of characters that precede the
 terminating NUL character.  The strnlen() function returns either the
 same result as strlen() or maxlen, whichever is smaller.

Co interpretuję jako, że długość łańcucha nie obejmuje wartości null. Dlatego kopiuję len + 1 bajtów, aby uwzględnić wartość null, a test sprawdza, czy długość <rozmiar bufora - 2. Minus jeden, ponieważ bufor zaczyna się na pozycji 0, a minus kolejny, aby upewnić się, że jest miejsce dla null.

EDYCJA: Okazuje się, że rozmiar czegoś zaczyna się od 1, a dostęp zaczyna się od 0, więc -2 przedtem było niepoprawne, ponieważ zwróciłoby błąd dla czegokolwiek> 98 bajtów, ale powinno być> 99 bajtów.

EDYCJA: Chociaż odpowiedź na temat skrótu bez znaku jest ogólnie poprawna, ponieważ maksymalna długość, którą można przedstawić, to 65 535 znaków, nie ma to większego znaczenia, ponieważ jeśli ciąg jest dłuższy, wartość zawinie się. To tak, jakby wziąć 75,231 (co jest 0x000125DF) i zamaskować górne 16 bitów, dając 9695 (0x000025DF). Jedynym problemem, jaki widzę w tym przypadku, jest pierwsze 100 znaków po 65 535, ponieważ sprawdzenie długości pozwoli na kopiowanie, ale we wszystkich przypadkach skopiuje tylko do pierwszych 100 znaków ciągu i zeruje ciąg . Więc nawet w przypadku problemu zawijania, bufor nadal nie zostanie przepełniony.

Może to samo w sobie stanowić zagrożenie bezpieczeństwa, w zależności od zawartości ciągu i tego, do czego go używasz. Jeśli jest to zwykły tekst, który jest czytelny dla człowieka, to generalnie nie ma problemu. Otrzymujesz po prostu obcięty ciąg. Jeśli jednak jest to adres URL lub nawet sekwencja poleceń SQL, możesz mieć problem.

Nawet jeśli używasz strncpy, długość odcięcia jest nadal zależna od przekazanego wskaźnika ciągu. Nie masz pojęcia, jak długi jest ten ciąg (to znaczy lokalizacja terminatora zerowego względem wskaźnika). Więc dzwonienie w strlenpojedynkę otwiera cię na słabość. Jeśli chcesz być bezpieczniejszy, użyj strnlen(str, 100).

Pełny kod poprawiony byłby:

int func(char *str) {
   char buffer[100];
   unsigned short len = strnlen(str, 100); // sizeof buffer

   if (len >= 100) {
     return -1;
   }

   strcpy(buffer, str); // this is safe since null terminator is less than 100th index
   return 0;
}

Odpowiedź z opakowaniem jest prawidłowa. Ale jest problem, o którym myślę, że nie został wspomniany, jeśli (len> = 100)

Cóż, gdyby Len miał 100, skopiowalibyśmy 100 elementów i nie mielibyśmy końcowych \ 0. To oczywiście oznaczałoby, że każda inna funkcja zależna od prawidłowo zakończonego łańcucha wyszłaby daleko poza oryginalną tablicę.

Ciąg problematyczny z C jest nierozwiązywalny IMHO. Zawsze lepiej jest mieć pewne ograniczenia przed rozmową, ale nawet to nie pomoże. Nie ma sprawdzania granic, więc przepełnienia bufora zawsze mogą i niestety się zdarzają ....

Poza kwestiami bezpieczeństwa związanymi z wywołaniem strlenwięcej niż jeden raz, generalnie nie należy używać metod łańcuchowych na łańcuchach, których długość jest dokładnie znana [w przypadku większości funkcji łańcuchowych istnieje tylko bardzo wąski przypadek, w którym należy ich używać - na łańcuchach, dla których maksymalna długość można zagwarantować, ale dokładna długość nie jest znana]. Gdy znana jest długość ciągu wejściowego i znana jest długość bufora wyjściowego, należy dowiedzieć się, jak duży region powinien zostać skopiowany, a następnie użyć go memcpy()do faktycznego wykonania danej kopii. Chociaż możliwe jest, że strcpymoże memcpy()to osiągnąć lepsze wyniki podczas kopiowania ciągu o wielkości zaledwie 1-3 bajtów, na wielu platformach memcpy()może być ponad dwukrotnie szybszy w przypadku większych ciągów.

Chociaż istnieją sytuacje, w których bezpieczeństwo kosztowałoby wydajność, jest to sytuacja, w której bezpieczne podejście jest również szybsze. W niektórych przypadkach rozsądne może być napisanie kodu, który nie jest zabezpieczony przed dziwnie zachowującymi się danymi wejściowymi, jeśli kod dostarczający dane wejściowe może zapewnić, że będą one dobrze zachowane i jeśli ochrona przed niewłaściwymi danymi wejściowymi zmniejszy wydajność. Zapewnienie, że długości łańcuchów są sprawdzane tylko raz, poprawia zarówno wydajność, jak i bezpieczeństwo, chociaż można zrobić jedną dodatkową rzecz, aby pomóc chronić bezpieczeństwo nawet podczas ręcznego śledzenia długości ciągu: dla każdego łańcucha, który ma mieć końcowe null, zapisz końcowe null jawnie, a nie niż oczekiwanie, że łańcuch źródłowy go będzie miał. Tak więc, gdyby ktoś pisał strdupodpowiednik:

char *strdupe(char const *src)
{
  size_t len = strlen(src);
  char *dest = malloc(len+1);
  // Calculation can't wrap if string is in valid-size memory block
  if (!dest) return (OUT_OF_MEMORY(),(char*)0); 
  // OUT_OF_MEMORY is expected to halt; the return guards if it doesn't
  memcpy(dest, src, len);      
  dest[len]=0;
  return dest;
}

Zauważ, że ostatnia instrukcja mogłaby zostać pominięta, jeśli memcpy przetworzyłby len+1bajty, ale jeśli inny wątek miałby zmodyfikować łańcuch źródłowy, wynikiem może być ciąg docelowy niezakończony NUL.