S5145 FP: User controlled data

[Aleksandr_Kovalev]

• Version 9.2.1 (build 49989)

if we wrap the check with in class method

var formName = AssertFormName(context.Request.Params["FormName"]);

...

private string AssertFormName(string formName)
        {
            if (string.IsNullOrEmpty(formName))
            {
                throw new BadRequestException("Form name is empty");
            }
            //TODO leave here till SQ fix
            return formName.Replace('\n', '_').Replace('\r', '_').Replace('\t', '_');
        }

the issue will not be reported, if we change it to extention

public static string RemoveDangerousCharacters(this string value)
        {
            if (string.IsNullOrEmpty(value))
            {
                return value;
            }
            return value.Replace('\n', '_').Replace('\r', '_').Replace('\t', '_');
        }

& call 

context.Request.Params["FormName"].RemoveDangerousCharacters()

the ussie is reported

P.S - we go through the input to leave just A-z0-9_ the issue is also reported

[Aleksandr_Kovalev]

if put method to the base class, the issue will be reported

 file = new UploadManagerService(**ProtectUserInput**(schema)).FileDownload(id);

[Caba_Sagi (Čaba Šagi)]

Hello @Aleksandr_Kovalev

Could you please tell me which rule is reporting the false positive?

All the best,
Čaba

[Aleksandr_Kovalev]

@Aleksandr_Kovalev

not sure where to see it, please take a look screen

[Caba_Sagi (Čaba Šagi)]

Hello @Aleksandr_Kovalev,

Thank you for the screenshot it helped me finding out what rule you were referring to (S5145).
So what is happening here is that in the RemoveDangerousCharacters(string) in case the string is null or empty you return the same value while in the AssertFormName(string) method it throws an exception. In the RemoveDangerousCharacters(string) our engine interprets it as a potentially unsanitized value. We are planning on improving the engine so it does not make this mistake in the future. Unfortunately I can not give you an ETA for when this support will be implemented.
As a workaround you should be able to return a null or empty string in the case it is null or empty and that should help you in fixing the false positive.

[Aleksandr_Kovalev]

got you, thx