Comment nous avons trouvé une vulnérabilité critique d'AspNetCore.Mvc et sommes passés à notre propre sérialisation

Bonjour, Habr!

Dans cet article, nous souhaitons partager notre expérience dans l'optimisation des performances et l'exploration des fonctionnalités d'AspNetCore.Mvc.



Contexte


Il y a plusieurs années, sur l'un de nos services chargés, nous avons constaté une consommation importante de ressources CPU. Cela semblait étrange, car la tâche du service consistait à prendre le message et à le mettre dans la file d'attente, après avoir effectué certaines opérations sur celui-ci, telles que la validation, l'ajout de données, etc.

À la suite du profilage, nous avons constaté que la désérialisation «mange» la plupart du temps du processeur. Nous avons jeté le sérialiseur standard et écrit le nôtre sur Jil, à la suite de quoi la consommation de ressources a diminué plusieurs fois. Tout a fonctionné comme il se doit et nous avons réussi à l'oublier.

Le problème


Nous améliorons constamment notre service dans tous les domaines, y compris la sécurité, les performances et la tolérance aux pannes, de sorte que «l'équipe de sécurité» effectue divers tests de nos services. Et il y a quelque temps, une alerte à propos d'une erreur dans le journal «vole» pour nous - nous avons en quelque sorte manqué un message non valide à son tour.

Avec une analyse détaillée, tout semblait plutôt étrange. Il existe un modèle de requête (ici je vais donner un exemple de code simplifié):

public class RequestModel { public string Key { get; set; } FromBody] Required] public PostRequestModelBody Body { get; set; } } public class PostRequestModelBody { [Required] [MinLength(1)] public IEnumerable<long> ItemIds { get; set; } } 

Il existe un contrôleur avec l'action Post, par exemple:

 [Route("api/[controller]")] public class HomeController : Controller { [HttpPost] public async Task<ActionResult> Post(RequestModel request) { if (this.ModelState.IsValid) { return this.Ok(); } return this.BadRequest(); } } 

Tout semble logique. Si une demande provient de la vue Corps

 {"itemIds":["","","" … ] } 

L'API renverra BadRequest, et il existe des tests pour cela.

Néanmoins, dans le journal, nous voyons le contraire. Nous avons pris le message des journaux, l'avons envoyé à l'API et avons obtenu le statut OK ... et ... une nouvelle erreur dans le journal. Ne croyant pas nos yeux, nous avons fait une erreur et nous sommes assurés que oui, en effet ModelState.IsValid == true. Dans le même temps, ils ont remarqué un temps d'exécution de requête inhabituellement long d'environ 500 ms, tandis que le temps de réponse habituel dépasse rarement 50 ms et celui-ci est en production, qui sert des milliers de requêtes par seconde!

La différence entre cette demande et nos tests était seulement que la demande contenait plus de 600 lignes vides ...

Ensuite, il y aura beaucoup de code bukaf.

Raison


Ils ont commencé à comprendre ce qui n'allait pas. Pour éliminer l'erreur, ils ont écrit une application propre (à partir de laquelle j'ai donné un exemple), avec laquelle nous avons reproduit la situation décrite. Au total, nous avons passé quelques jours-homme sur la recherche, les tests, le code de débogage mental AspNetCore.Mvc et il s'est avéré que le problème était dans JsonInputFormatter .

JsonInputFormatter utilise un JsonSerializer qui, en obtenant un flux pour la désérialisation et le type, essaie de sérialiser chaque propriété, s'il s'agit d'un tableau - chaque élément de ce tableau. Dans le même temps, si une erreur se produit pendant la sérialisation, JsonInputFormatter enregistre chaque erreur et son chemin, la marque comme traitée, afin que vous puissiez poursuivre la tentative de désérialisation.

Voici le code du gestionnaire d'erreurs JsonInputFormatter (il est disponible sur Github au lien ci-dessus):

 void ErrorHandler(object sender, Newtonsoft.Json.Serialization.ErrorEventArgs eventArgs) { successful = false; // When ErrorContext.Path does not include ErrorContext.Member, add Member to form full path. var path = eventArgs.ErrorContext.Path; var member = eventArgs.ErrorContext.Member?.ToString(); var addMember = !string.IsNullOrEmpty(member); if (addMember) { // Path.Member case (path.Length < member.Length) needs no further checks. if (path.Length == member.Length) { // Add Member in Path.Memb case but not for Path.Path. addMember = !string.Equals(path, member, StringComparison.Ordinal); } else if (path.Length > member.Length) { // Finally, check whether Path already ends with Member. if (member[0] == '[') { addMember = !path.EndsWith(member, StringComparison.Ordinal); } else { addMember = !path.EndsWith("." + member, StringComparison.Ordinal); } } } if (addMember) { path = ModelNames.CreatePropertyModelName(path, member); } // Handle path combinations such as ""+"Property", "Parent"+"Property", or "Parent"+"[12]". var key = ModelNames.CreatePropertyModelName(context.ModelName, path); exception = eventArgs.ErrorContext.Error; var metadata = GetPathMetadata(context.Metadata, path); var modelStateException = WrapExceptionForModelState(exception); context.ModelState.TryAddModelError(key, modelStateException, metadata); _logger.JsonInputException(exception); // Error must always be marked as handled // Failure to do so can cause the exception to be rethrown at every recursive level and // overflow the stack for x64 CLR processes eventArgs.ErrorContext.Handled = true; } 

Les erreurs de marquage comme traitées sont faites à la toute fin du processeur

 eventArgs.ErrorContext.Handled = true; 


Ainsi, une fonctionnalité est implémentée pour générer toutes les erreurs de désérialisation et les chemins d'accès, sur quels champs / éléments ils étaient, enfin ... presque tous ...

Le fait est que JsonSerializer a une limite de 200 erreurs, après quoi il se bloque, tandis que tout le code se bloque et ModelState devient ... valide! ... les erreurs sont également perdues.

Solution


Sans hésitation, nous avons implémenté notre formateur Json pour Asp.Net Core en utilisant le Jil Deserializer. Étant donné que le nombre d'erreurs dans le corps n'est absolument pas important pour nous, seul le fait de leur présence est important (et il est généralement difficile d'imaginer une situation où cela serait vraiment utile), le code sérialiseur s'est avéré assez simple.

Je vais donner le code principal de JilJsonInputFormatter personnalisé:

 using (var reader = context.ReaderFactory(request.Body, encoding)) { try { var result = JSON.Deserialize( reader: reader, type: context.ModelType, options: this.jilOptions); if (result == null && !context.TreatEmptyInputAsDefaultValue) { return await InputFormatterResult.NoValueAsync(); } else { return await InputFormatterResult.SuccessAsync(result); } } catch { // -   } return await InputFormatterResult.FailureAsync(); } 

Attention! Jil est sensible à la casse, ce qui signifie le contenu du corps

 {"ItemIds":["","","" … ] } 

et

 {"itemIds":["","","" … ] } 

pas la même chose. Dans le premier cas, si camelCase est utilisé, la propriété ItemIds sera nulle après la désérialisation.

Mais comme il s'agit de notre API, nous l'utilisons et la contrôlons, pour nous ce n'est pas critique. Le problème peut survenir s'il s'agit d'une API publique et que quelqu'un l'appelle déjà, en passant le nom du paramètre pas dans camelCase.

Résultat


Le résultat a même dépassé nos attentes, l'API devrait commencer à renvoyer BadRequest à la demande demandée et l'a fait très rapidement. Vous trouverez ci-dessous des captures d'écran de certains de nos graphiques, qui montrent clairement les changements dans le temps de réponse et le processeur, avant et après le déploiement.
Délai de demande:

image

Vers 16 h, il y a eu un déploiement. Avant le déploiement, le temps d'exécution de p99 était de 30 à 57 ms, après le déploiement, il est devenu de 9 à 15 ms. (Vous ne pouvez pas faire attention aux pics répétés de 18h00 - c'était un autre déploiement)

Voici comment le graphique du CPU a changé:

image

Pour cette raison, nous avons apporté un problème à Github, au moment de la rédaction, il a été signalé comme bogue avec le jalon 3.0.0-preview3.

En conclusion


Jusqu'à ce que le problème soit résolu, nous pensons qu'il est préférable d'abandonner l'utilisation de la désérialisation standard, surtout si vous avez une API publique. Connaissant ce problème, un attaquant peut facilement y mettre votre service, y envoyant un tas de requêtes invalides similaires, car plus le tableau erroné est grand, plus il y a de corps, plus le traitement se déroule dans JsonInputFormatter.

Artyom Astashkin, chef d'équipe de développement

Source: https://habr.com/ru/post/fr435626/


All Articles