int func(char* str)
{
char buffer[100];
unsigned short len = strlen(str);
if(len >= 100)
{
return (-1);
}
strncpy(buffer,str,strlen(str));
return 0;
}
Ce code est vulnérable à une attaque par débordement de tampon, et j'essaie de comprendre pourquoi. Je pense que cela a à voir avec le fait d' len
être déclaré un short
au lieu d'un int
, mais je ne suis pas vraiment sûr.
Des idées?
c
security
buffer-overflow
Jason
la source
la source
strncpy
. Dans ce cas, ce n'est pas le cas.strlen
est calculé, utilisé pour le contrôle de validité, puis il est à nouveau calculé de manière absurde - c'est un échec DRY. Si le secondstrlen(str)
était remplacé parlen
, il n'y aurait aucune possibilité de débordement de tampon, quel que soit le type delen
. Les réponses n'abordent pas ce point, elles parviennent simplement à l'éviter.Réponses:
Sur la plupart des compilateurs, la valeur maximale de an
unsigned short
est 65535.Toute valeur au-dessus de cela est bouclée, donc 65536 devient 0 et 65600 devient 65.
Cela signifie que de longues chaînes de la bonne longueur (par exemple 65600) passeront le contrôle et déborderont de mémoire tampon.
Permet
size_t
de stocker le résultat destrlen()
, nonunsigned short
et de le comparerlen
à une expression qui code directement la taille debuffer
. Donc par exemple:la source
len
comme troisième argument de strncpy. Utiliser à nouveau strlen est stupide dans tous les cas./ sizeof(buffer[0])
- notez quesizeof(char)
dans C est toujours 1 (même si un caractère contient un milliard de bits) donc c'est superflu quand il n'y a pas de possibilité d'utiliser un type de données différent. Encore ... bravo pour une réponse complète (et merci d'avoir été réactif aux commentaires).char[]
etchar*
ne sont pas la même chose. Il existe de nombreuses situations dans lesquelles unchar[]
will est implicitement converti en un fichierchar*
. Par exemple,char[]
est exactement le même quechar*
lorsqu'il est utilisé comme type d'arguments de fonction. Cependant, la conversion ne se produit pas poursizeof()
.buffer
à un moment donné, l'expression se met à jour automatiquement. Ceci est essentiel pour la sécurité, car la déclaration debuffer
peut être à quelques lignes de l'enregistrement du code réel. Il est donc facile de changer la taille du tampon, mais oubliez de mettre à jour à chaque emplacement où la taille est utilisée.Le problème est ici:
Si la chaîne est supérieure à la longueur du tampon cible, strncpy la copiera toujours. Vous basez le nombre de caractères de la chaîne comme le nombre à copier au lieu de la taille du tampon. La bonne façon de procéder est la suivante:
Cela limite la quantité de données copiées à la taille réelle de la mémoire tampon moins un pour le caractère de fin nul. Ensuite, nous définissons le dernier octet du tampon sur le caractère nul en tant que sauvegarde supplémentaire. La raison en est que strncpy copiera jusqu'à n octets, y compris le null de fin, si strlen (str) <len - 1. Sinon, le null n'est pas copié et vous avez un scénario de plantage car maintenant votre tampon a un non terminé chaîne.
J'espère que cela t'aides.
EDIT: Après un examen plus approfondi et des contributions d'autres personnes, un codage possible pour la fonction suit:
Puisque nous connaissons déjà la longueur de la chaîne, nous pouvons utiliser memcpy pour copier la chaîne de l'emplacement référencé par str dans le tampon. Notez que d'après la page de manuel de strlen (3) (sur un système FreeBSD 9.3), ce qui suit est indiqué:
Ce que j'interprète comme étant que la longueur de la chaîne n'inclut pas le null. C'est pourquoi je copie len + 1 octets pour inclure la valeur null, et le test vérifie que la longueur <taille du tampon - 2. Moins un car le tampon commence à la position 0, et moins un autre pour s'assurer qu'il y a de la place pour le nul.
EDIT: Il s'avère que la taille de quelque chose commence par 1 tandis que l'accès commence par 0, donc le -2 avant était incorrect car il renverrait une erreur pour tout ce qui> 98 octets mais il devrait être> 99 octets.
EDIT: Bien que la réponse à propos d'un court non signé soit généralement correcte car la longueur maximale pouvant être représentée est de 65 535 caractères, cela n'a pas vraiment d'importance car si la chaîne est plus longue que cela, la valeur s'enroulera. C'est comme prendre 75,231 (qui est 0x000125DF) et masquer les 16 premiers bits vous donnant 9695 (0x000025DF). Le seul problème que je vois avec ceci est les 100 premiers caractères après 65 535 car la vérification de la longueur autorisera la copie, mais elle ne copiera que les 100 premiers caractères de la chaîne dans tous les cas et la valeur null terminera la chaîne . Ainsi, même avec le problème de bouclage, le tampon ne sera toujours pas débordé.
Cela peut ou non poser en soi un risque de sécurité en fonction du contenu de la chaîne et de l'utilisation que vous en faites. S'il ne s'agit que d'un texte simple lisible par l'homme, il n'y a généralement pas de problème. Vous obtenez juste une chaîne tronquée. Cependant, si c'est quelque chose comme une URL ou même une séquence de commandes SQL, vous pourriez avoir un problème.
la source
func
... et toutes les autres fonctions C jamais écrites qui prennent des chaînes terminées par NUL comme arguments. Évoquer la possibilité que l'entrée ne soit pas terminée par NUL est complètement inutile.len >= 100
) a été effectué contre une valeur mais que la longueur de la copie a reçu une valeur différente ... est une violation du principe DRY. L'appel simplestrncpy(buffer, str, len)
évite la possibilité de débordement de tampon et fait moins de travail questrncpy(buffer,str,sizeof(buffer) - 1)
... bien qu'ici, ce soit juste un équivalent plus lent dememcpy(buffer, str, len)
.Même si vous utilisez
strncpy
, la longueur de la coupure dépend toujours du pointeur de chaîne passé. Vous n'avez aucune idée de la longueur de cette chaîne (l'emplacement du terminateur nul par rapport au pointeur, c'est-à-dire). Appelerstrlen
seul vous ouvre donc à la vulnérabilité. Si vous voulez être plus sûr, utilisezstrnlen(str, 100)
.Le code complet corrigé serait:
la source
strlen
également accès au-delà de la fin du tampon?strnlen
ne résout pas le problème si ce que orlp suggère est supposé correct de toute façon.buffer
. "puisque str pourrait pointer vers un tampon de 2 octets, dont aucun n'est NUL." - ce n'est pas pertinent, comme c'est le cas pour toute implémentation defunc
. La question ici concerne le dépassement de tampon, pas UB car l'entrée n'est pas terminée par NUL.La réponse avec l'emballage est juste. Mais il y a un problème qui, je pense, n'a pas été mentionné si (len> = 100)
Eh bien, si Len valait 100, nous copierions 100 éléments et nous n'aurions pas de \ 0 à la fin. Cela signifierait clairement que toute autre fonction dépendant de la chaîne correctement terminée marcherait bien au-delà du tableau d'origine.
La chaîne problématique de C est insoluble à mon humble avis. Vous feriez toujours mieux d'avoir des limites avant l'appel, mais même cela n'aidera pas. Il n'y a pas de vérification des limites et donc les débordements de tampon peuvent toujours et se produiront malheureusement
la source
strncpy()
et amis, mais la mémoire allouant des fonctions commestrdup()
et amis. Ils sont dans la norme POSIX-2008, ils sont donc assez portables, bien qu'ils ne soient pas disponibles sur certains systèmes propriétaires.buffer
est locale à cette fonction et n'est pas utilisée ailleurs. Dans un programme réel, nous devrions examiner comment il est utilisé ... parfois, la terminaison NUL n'est pas correcte (l'utilisation originale de strncpy était de créer des entrées de répertoire de 14 octets UNIX - remplies par NUL et non terminées par NUL). «La chaîne problématique de C est insoluble à mon humble avis» - alors que C est un langage génial qui a été surpassé par une technologie bien meilleure, un code sûr peut y être écrit si une discipline suffisante est utilisée.if (len >= 100)
est la condition pour quand la vérification échoue , pas quand elle réussit, ce qui signifie qu'il n'y a pas de cas où exactement 100 octets sans terminateur NUL sont copiés, car cette longueur est incluse dans la condition d'échec.Au-delà des problèmes de sécurité liés à l'appel
strlen
plus d'une fois, il ne faut généralement pas utiliser de méthodes de chaîne sur des chaînes dont la longueur est précisément connue [pour la plupart des fonctions de chaîne, il n'y a qu'un cas vraiment étroit où elles doivent être utilisées - sur des chaînes pour lesquelles un maximum la longueur peut être garantie, mais la longueur précise n'est pas connue]. Une fois que la longueur de la chaîne d'entrée est connue et que la longueur du tampon de sortie est connue, il faut déterminer la taille d'une région à copier et l'utiliser ensuitememcpy()
pour effectuer réellement la copie en question. Bien qu'il soit possible questrcpy
cela soit plus performantmemcpy()
lors de la copie d'une chaîne de seulement 1 à 3 octets ou plus, sur de nombreuses platesmemcpy()
- formes, il est susceptible d'être plus de deux fois plus rapide lorsqu'il s'agit de chaînes plus volumineuses.Bien qu'il existe certaines situations où la sécurité se ferait au détriment des performances, c'est une situation où l'approche sécurisée est également la plus rapide. Dans certains cas, il peut être raisonnable d'écrire du code qui n'est pas sécurisé contre les entrées au comportement étrange, si le code fournissant les entrées peut garantir qu'elles se comportent correctement, et si la protection contre les entrées mal conduites entraverait les performances. S'assurer que les longueurs de chaîne ne sont vérifiées qu'une seule fois améliore à la fois les performances et la sécurité, bien qu'une chose supplémentaire puisse être faite pour aider à protéger la sécurité même lors du suivi manuel de la longueur de la chaîne: pour chaque chaîne qui devrait avoir un nul à la fin, écrivez plutôt le null à la fin. que de s'attendre à ce que la chaîne source l'ait. Ainsi, si l'on écrivait un
strdup
équivalent:Notez que la dernière instruction pourrait généralement être omise si le memcpy avait traité des
len+1
octets, mais si un autre thread devait modifier la chaîne source, le résultat pourrait être une chaîne de destination non terminée par NUL.la source
strlen
plus d'une fois ?strlen
et effectué une action en fonction de la valeur renvoyée (ce qui était probablement la raison de l'appel en premier lieu), un appel répété (1) donnera toujours la même réponse que le premier, dans ce cas, c'est simplement du travail gaspillé, ou (2) peut parfois (parce que quelque chose d'autre - peut-être un autre thread - a modifié la chaîne entre-temps) donner une réponse différente, auquel cas le code qui fait certaines choses avec la longueur (par exemple allouer un tampon) peut prendre une taille différente de celle du code qui fait autre chose (copie dans le tampon).