Search code examples
c#asp.net-corepost

Prevent user manipulation of POST requests in ASP.NET Core


I have a simple ASP.NET Core Razor pages site in which a user inputs a room number and it outputs a result whether it is available or not.

After finishing the development, it was submitted to the security team and we got this message:

Issue is existing on the POST side, when we're manipulating the request, we can pass by special characters (raw) or URL encoded.

This is part of the Index.cshtml

<input class="form-control" asp-for="InputRoomNo" pattern="[a-zA-Z0-9]+" title="Only charcters and letters please" minlength="3" maxlength="5">

This is most of Index.cshtml.cs

public class IndexModel : PageModel
{
    public readonly IConfiguration config;
    [BindProperty(SupportsGet = false)]
    public string InputRoomNo { get; set; }
    public Room roomInfo = null;
    public string conn;
    
    public IndexModel(IConfiguration Configuration)
    {
        config = Configuration;
    }
        public void OnPost()
        {
            conn = config.GetConnectionString("ConnectionString");
            string b = $"{InputRoomNo}";
            b = Request.Form["inputRoomNo"];
            roomInfo = Search($"{InputRoomNo}");
        }

        public void OnGet()
        {
        }

The Search is a method that opens a connection to the database and does a select.

What is wrong and how can the security concern be addressed?

UPDATE: After applying the suggested answer, I got this response: Issue is still existing while manipulating the request, special characters (including space (%20) is passing) It could be bypassed using URL encoding or intercepting the request and sending special characters within 5 characters.


Solution

  • Looks like you're getting concepts mixed up so it might do you good to simplify.

    1. You already have InputRoomNo as a BindProperty meaning the OnPost method can read the value from there
    2. As a result, it's unnecessary to make use of Request.Form since you can reference the value from your property.
    3. It's unclear what b is used for

    Your code could be simplified as follows (unless b is used in code that wasn't included in your post):

    public void OnPost()
    {
        roomInfo = Search(InputRoomNo);
    }
    

    By accessing the value directly from the public BindProperty, you're bypassing the potential manipulation of the request form data, but don't forget to properly sanitize and do necessary validation on data before you use it.

    PS. Since it's the Search method that opens the database connection to select data, you don't need the connection in OnPost either.

    In fact, you could actually abstract this out to a named page handler in which case your search function would change name from Search to OnPostSearch or OnPostSearchAsync if you're doing asynchronous operations in it.

    public async Task OnPostSearchAsync()
    {
        conn = config.GetConnectionString("ConnectionString");
        // Do your search logic here.
    
        roomInfo = ...; // Assign roomInfo to your search results
    }
    
    Your form would change:
    
        <form method="post">
    
    Would become
    
         <form method="post" asp-page-handler="Search">