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.
Looks like you're getting concepts mixed up so it might do you good to simplify.
InputRoomNo
as a BindProperty
meaning the OnPost
method can read the value from thereRequest.Form
since you can reference the value from your property.b
is used forYour 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">