ASP.NET MVC UpdateModel vulnérable au piratage?

Question

J'ai une application ASP.NET MVC qui ressemble à un calendrier. Comme dans l'exemple de NerdDinner, je mets à jour les résultats de ma page d'édition en utilisant UpdateMethod()

Dans mon application, certains événements sont entièrement personnalisables et certains sont seulement partiellement personnalisables. Même si le formulaire d'édition pour modifier les événements partiellement personnalisables n'a que ces champs disponibles, il est évident que quelqu'un pourrait créer son propre formulaire avec les données manquantes et les publier sur mon site. S'ils le font, qu'est-ce qui empêchera quelqu'un de changer tout ou partie des champs? Pire, et s'ils essayaient de changer l'identifiant (clé primaire)?

Il semble que UpdateModel() soit vulnérable au piratage très basique. Mes peurs sont-elles légitimes ou y a-t-il quelque chose qui me manque?

// POST: /MyEvents/Edit/2 
[AcceptVerbs(HttpVerbs.Post), Authorize] 
public ActionResult Edit(int id, FormCollection formValues) 
{ 
    MyEvent myevent = eventRepository.GetMyEvent(id); 

    try 
    { 
     UpdateModel(myevent); 
     eventRepository.Save(); 
     return RedirectToAction("Details", new { id = myevent.MyEventId }); 
    } 
    catch 
    { 
     ModelState.AddRuleViolations(myevent.GetRuleViolations()); 
     return View(new MyEventFormViewModel(myevent)); 
    } 
} 

Answer 1

Il vous manque la section "Modèle de sécurité de liaison". Vous devez toujours inclure une liste blanche de propriétés pouvant être mises à jour par l'une de vos méthodes de saisie utilisateur.

Par exemple, à partir NerdDinner:

[AcceptVerbs(HttpVerbs.Post)] 
public ActionResult Create([Bind(Include="Title, Address")] Dinner dinner) 
{ 

} 

ou si vous appelez UpdateModel, vous pouvez créer un tableau de chaînes de propriétés autorisés, et faire

UpdateModel(myObject, allowedProperties); 

Vous pouvez verrouiller les classes eux-mêmes de sorte que seules certaines propriétés sont également mises à jour.

[Bind(Include="MyProp1,MyProp2,MyProp3")] 
public partial class MyEntity { } 

Answer 2

Vos peurs sont bonnes. C'est ce qu'on appelle l'assignation de masse. Vous pouvez protéger votre code en marquant votre classe avec BindAttribute et en définissant les propriétés Exclude/Include.

Answer 3

Il existe des surcharges de UpdateModel qui prennent un tableau de propriétés de dénomination de chaînes à mettre à jour. Ces surcharges ne mettront à jour que les propriétés nommées.

Il peut y avoir d'autres moyens plus faciles/plus déclaratifs pour accomplir cela, je ne suis pas un expert en liaison de données MVC.

Answer 4

Vous pouvez marquer des champs sur votre modèle qui doivent être ignorés par la mise à jour ou transmettre une liste de champs inclus/exclus en utilisant l'une des autres surcharges UpdateModel.

Answer 5

Il est parfaitement possible pour quelqu'un d'entreprenant/malveillant de mapper les champs à l'une des propriétés de votre modèle. Il y a plusieurs façons de contourner cela

Le plus simple est d'utiliser les surcharges de propriétés exclude/include de UpdateModel comme cela a été mentionné précédemment. L'inconvénient de ceci est que la méthode accepte seulement un tableau de chaînes qui peut parfois signifier que votre code est désynchronisé si vous changez de nom. Une autre façon est d'utiliser un simple DTO qui contient les champs liés, vous pouvez ensuite prendre le DTO et faire ce que vous voulez avec votre objet événement, cela ajoute évidemment une autre classe et est beaucoup plus manuel mais vous donne beaucoup plus contrôle

public ActionResult(int id, EditForm form) { 
    MyEvent event = _eventRepository.GetMyEvent(id); 
    event.Name = form.Name; //etc; 
    if (User.IsInRole("Organiser")) { 
     event.Date = form.Date; 
    } 
    return ... 
} 

une autre façon pourrait être au moyen d'un modèle de liaison client pour votre classe MyEvent qui ne lie que votre champ désiré, probablement surpuissant cependant.