Por que esse código é vulnerável a ataques de estouro de buffer?

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

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

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

Esse código é vulnerável a um ataque de estouro de buffer e estou tentando descobrir o porquê. Estou pensando que tem a ver com lenser declarado um em shortvez de um int, mas não tenho muita certeza.

Alguma ideia?

Na maioria dos compiladores, o valor máximo de um unsigned shorté 65535.

Qualquer valor acima que é contornado, então 65536 se torna 0 e 65600 se torna 65.

Isso significa que seqüências longas do comprimento certo (por exemplo, 65600) passam na verificação e sobrecarregam o buffer.

Use size_tpara armazenar o resultado de strlen(), não unsigned shorte comparar lencom uma expressão que codifica diretamente o tamanho de buffer. Então, por exemplo:

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

O problema está aqui:

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

Se a string for maior que o comprimento do buffer de destino, o strncpy ainda a copiará. Você está baseando o número de caracteres da sequência como o número a ser copiado em vez do tamanho do buffer. A maneira correta de fazer isso é a seguinte:

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

O que isso faz é limitar a quantidade de dados copiados para o tamanho real do buffer menos um para o caractere final nulo. Em seguida, definimos o último byte no buffer para o caractere nulo como uma proteção adicional. A razão para isso é que o strncpy copiará até n bytes, incluindo o nulo final, se strlen (str) <len - 1. Caso contrário, o nulo não será copiado e você terá um cenário de falha, porque agora seu buffer não possui terminação. corda.

Espero que isto ajude.

EDIT: Após um exame mais aprofundado e contribuições de outras pessoas, segue uma codificação possível para a função:

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);
  }

Como já sabemos o comprimento da string, podemos usar o memcpy para copiar a string do local referenciado por str no buffer. Observe que, na página de manual do strlen (3) (em um sistema FreeBSD 9.3), o seguinte é indicado:

 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.

O que eu interpreto é que o comprimento da string não inclui o nulo. É por isso que copio len + 1 bytes para incluir o nulo, e o teste verifica se o comprimento <tamanho do buffer - 2. Menos um porque o buffer inicia na posição 0 e menos outro para garantir que haja espaço para o nulo.

EDIT: Acontece que o tamanho de algo começa com 1 enquanto o acesso começa com 0; portanto, o -2 anterior estava incorreto porque retornaria um erro para qualquer coisa> 98 bytes, mas deveria ser> 99 bytes.

EDIT: Embora a resposta sobre um curto não assinado esteja geralmente correta, já que o comprimento máximo que pode ser representado é 65.535 caracteres, isso realmente não importa, porque se a string for maior que isso, o valor será contornado. É como pegar 75.231 (que é 0x000125DF) e mascarar os 16 bits principais, fornecendo 9695 (0x000025DF). O único problema que vejo com isso são os primeiros 100 caracteres após 65.535, pois a verificação de comprimento permitirá a cópia, mas copiará até os 100 primeiros caracteres da string em todos os casos e terminará nula . Portanto, mesmo com o problema abrangente, o buffer ainda não será excedido.

Isso pode ou não representar, por si só, um risco de segurança, dependendo do conteúdo da string e para o que você a está usando. Se é apenas texto simples legível por humanos, geralmente não há problema. Você acabou de obter uma string truncada. No entanto, se for algo como uma URL ou mesmo uma sequência de comandos SQL, você poderá ter um problema.

Mesmo que você esteja usando strncpy, o comprimento do corte ainda depende do ponteiro da string passado. Você não tem idéia de quanto tempo essa cadeia é (a localização do terminador nulo em relação ao ponteiro, ou seja). Portanto, ligar strlensozinho abre a vulnerabilidade. Se você quer ser mais seguro, use strnlen(str, 100).

O código completo corrigido seria:

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;
}

A resposta com a embalagem está correta. Mas há um problema que acho que não foi mencionado se (len> = 100)

Bem, se Len tivesse 100, copiaríamos 100 elementos e não teríamos \ 0 à direita. Isso claramente significaria que qualquer outra função, dependendo da sequência final correta, andaria muito além da matriz original.

A string problemática de C é IMHO insolúvel. É melhor você sempre ter alguns limites antes da ligação, mas mesmo isso não ajudará. Não há verificação de limites e, portanto, os estouros de buffer sempre podem e infelizmente acontecerão ....

Além dos problemas de segurança envolvidos na chamada strlenmais de uma vez, geralmente não se deve usar métodos de string em strings cujo comprimento é precisamente conhecido [para a maioria das funções de strings, existe apenas um caso muito restrito em que elas devem ser usadas - em strings para as quais um máximo o comprimento pode ser garantido, mas o comprimento exato não é conhecido]. Uma vez que o comprimento da string de entrada é conhecido e o tamanho do buffer de saída, deve-se descobrir qual o tamanho de uma região que deve ser copiada e, em seguida, usar memcpy()para realmente executar a cópia em questão. Embora seja possível que strcpytenha um desempenho superior memcpy()ao copiar uma sequência de apenas 1 a 3 bytes, em muitas plataformas memcpy()é provável que seja mais do que o dobro da velocidade ao lidar com sequências maiores.

Embora existam algumas situações em que a segurança teria um custo de desempenho, essa é uma situação em que a abordagem segura também é a mais rápida. Em alguns casos, pode ser razoável escrever código que não seja seguro contra entradas que se comportam de maneira estranha, se o código que fornece as entradas pode garantir que elas serão bem comportadas e se a proteção contra entradas mal comportadas impediria o desempenho. Garantir que os comprimentos de sequência sejam verificados apenas uma vez melhora o desempenho e a segurança, embora uma coisa extra possa ser feita para ajudar a proteger a segurança, mesmo ao rastrear manualmente o comprimento da sequência: para cada sequência que se espera que tenha um nulo final, escreva o nulo final explicitamente. do que esperar que a string de origem a tenha. Assim, se alguém estivesse escrevendo um strdupequivalente:

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;
}

Observe que a última instrução geralmente poderia ser omitida se o memcpy tivesse processado len+1bytes, mas outro segmento era modificar a cadeia de origem, o resultado poderia ser uma cadeia de destino não terminada por NUL.