Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SEC109 False Positive #25

Closed
kyleherzog opened this issue May 24, 2017 · 5 comments
Closed

SEC109 False Positive #25

kyleherzog opened this issue May 24, 2017 · 5 comments
Assignees
Labels
Milestone

Comments

@kyleherzog
Copy link

In VS2017 SEC0109 is being raised with the following example code, which pretty much mirrors what the documentation says to do to fix it.

        if (Url.IsLocalUrl(returnUrl))
        {
            return Redirect(returnUrl); //this line is flagged with SEC109
        }
@ejohn20 ejohn20 self-assigned this May 24, 2017
@ejohn20 ejohn20 added the bug label May 24, 2017
@ejohn20
Copy link
Member

ejohn20 commented May 24, 2017

Confirmed bug. Add the IsLocalUrl to the acceptable cleanse methods for SEC0109.

@ejohn20 ejohn20 added this to the 1.0.6 milestone May 24, 2017
@ejohn20
Copy link
Member

ejohn20 commented Jun 30, 2017

This will be addressed using a new code block analyzer for taint analysis within a block of code.

@serbentraut
Copy link

Same bug appears to apply to SEC110, which is a Web Forms redirect.

@ejohn20
Copy link
Member

ejohn20 commented Aug 1, 2017

Perfect, we'll make sure both rules are fixable using the code suggestions. For now, you can suppress them by right clicking the warning and adding them to the suppression file.

@ejohn20 ejohn20 modified the milestones: 1.0.7, 1.0.6 Aug 1, 2017
@ejohn20 ejohn20 modified the milestones: 1.0.7, 2.0 Jan 18, 2018
@ejohn20
Copy link
Member

ejohn20 commented Aug 15, 2018

In VS2017 SEC0109 is being raised with the following example code, which pretty much mirrors what the documentation says to do to fix it.

Confirmed this is fixed in 2.0 using the new code block analyzer. The method must be invoked on the tainted variable inside the same code block. The analyzers are not smart enough to traces this through multiple method calls.

Same bug appears to apply to SEC110, which is a Web Forms redirect.

The issues was also corrected by the code block analyzer. This code does not get flagged in the 2.0+ version of the analyzers:

Uri uri;
string url = Request.QueryString["returnUrl"];
if (Uri.TryCreate(url, UriKind.Relative, out uri))
{
      Response.Redirect(url);
}

@ejohn20 ejohn20 closed this as completed Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants