MSkidmore454594 (Community Member) asked a question.
Hi @MSkidmore454594 (Community Member),
Encrypting the ViewState would help to prevent users from intercepting and/or tampering with the state data, assuming you have configured a suitable machineKey as described here https://docs.microsoft.com/en-us/previous-versions/dotnet/netframework-4.0/w8h3skw9(v=vs.100)?redirectedfrom=MSDN). Encrypting the ViewState is recommended by Microsoft when working with sensitive data (https://docs.microsoft.com/en-us/previous-versions/dotnet/articles/aa479501(v%3dmsdn.10)) as a best-practice for defense-in-depth security.
Without looking at the specific flaw identified, it is hard to answer your question because the issue may still be present if the application stores a user-supplied value in the ViewState without prior and satisfactory cleansing. As such if that were the case, a potentially malicious input may be persisted in the ViewState (though now in an encrypted form), but then then reflected later on. As such it is likely that enabling ViewStateEncryptionMode="Always" will not solve the CWE 470 flaw, but it would improve the security posture of your application.
Veracode does not currently support cleansing methods specifically for CWE 470. The recommendation for remediation is to perform validation on the untrusted inputs. This should be done using a whitelist approach if possible, permitting only data that is known to be acceptable via for example a regular expression.
I recommend scheduling a consultation with a Veracode Application Security Consultant if you would like to discuss the specific mitigations further. You can find details on how to schedule a consultation here: https://help.veracode.com/reader/DGHxSJy3Gn3gtuSIN2jkRQ/7YQTCDJKFEQzL3gL_N90hQ.
Thanks,
Anthony Fielding
Thanks, Anthony,
The flaw is reported as:
The first argument to LoadControl() contains tainted data. The tainted data originated from an earlier call to system_web_dll.System.Web.UI.Control.get_ViewState.
in this line of code:
userControl = (System.Web.UI.UserControl)LoadControl("Some/" + Control + ".ascx");
Control is getting set in a Property:
public string Control
{
get
{
if (ViewState["SomeControl"] != null)
{
return ViewState["SomeControl"].ToString();
}
else
{
return "Comments";
}
}
set
{
ViewState["SomeControl"] = value;
}
}
I suppose, probably via a whitelist as you suggest, I need to sanitize what is being returned from the property? Yes?
Hi @MSkidmore454594 (Community Member),
Thanks for the context around the flaw you reported. In this case I can see that as the code stands, a malicious actor could potentially inject a payload that might cause the .NET runtime to execute unexpected code via reflection.
I recommend if possible you build and maintain an enumerable list of allowed control names that can be specified. This would form your whitelist - a case-sensitive list of permitted control names. This list could be constructed via hard coding, or through reflection during application initialisation or dynamically as required. The code should then test the value (ViewState["SomeControl"]) against matches in the enumeration in the get method and only return those matches or "Comments".
I hope that helps.
Thanks,
Anthony Fielding
.png){kind=logo}
Hi @MSkidmore454594 (Community Member),
Thanks for the context around the flaw you reported. In this case I can see that as the code stands, a malicious actor could potentially inject a payload that might cause the .NET runtime to execute unexpected code via reflection.
I recommend if possible you build and maintain an enumerable list of allowed control names that can be specified. This would form your whitelist - a case-sensitive list of permitted control names. This list could be constructed via hard coding, or through reflection during application initialisation or dynamically as required. The code should then test the value (ViewState["SomeControl"]) against matches in the enumeration in the get method and only return those matches or "Comments".
I hope that helps.
Thanks,
Anthony Fielding