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

No taint warning with lambda #4823

Closed
JarLob opened this issue Feb 10, 2021 · 2 comments · Fixed by #4907
Closed

No taint warning with lambda #4823

JarLob opened this issue Feb 10, 2021 · 2 comments · Fixed by #4907
Labels
Category-Security DataFlow Enhancement False_Negative No diagnostic is reported for a problematic case help wanted The issue is up-for-grabs, and can be claimed by commenting

Comments

@JarLob
Copy link
Contributor

JarLob commented Feb 10, 2021

Analyzer

CA3001, but probably all taint tracking analyzers

Analyzer source

Master branch

Describe the bug

No security warning (taint tracking issue?) when the sink is in lambda

Steps To Reproduce

Modify the existing DocSample1_CSharp_Violation_Diagnostic to:

using System;
using System.Data;
using System.Data.SqlClient;

namespace TestNamespace
{
    public partial class WebForm : System.Web.UI.Page
    {
        public static string ConnectionString { get; set; }

        protected void Page_Load(object sender, EventArgs e)
        {
            string name = Request.Form[""product_name""];
            using (SqlConnection connection = new SqlConnection(ConnectionString))
            {
                Func<string, SqlCommand> lambdaExpr = (x) => {
                    SqlCommand sqlCommand = new SqlCommand()
                    {
                        CommandText = ""SELECT ProductId FROM Products WHERE ProductName = '"" + x + ""'"",
                        CommandType = CommandType.Text,
                    };
                    return sqlCommand;
                };
                SqlCommand cmd = lambdaExpr(name);

                SqlDataReader reader = cmd.ExecuteReader();
            }
        }
    }
}

Expected behavior

Warning

Actual behavior

No warning. Test fails.

@Evangelink
Copy link
Member

@dotpaul are we able to track such case with the current dataflow? In my past job we had to implement a more complex flow parsing to handle such cases.

@dotpaul
Copy link
Contributor

dotpaul commented Feb 11, 2021

@Evangelink DataFlowOperationVisitor has a VisitInvocation_Lambda method, but TaintedDataOperationVisitor doesn't implement it. I don't remember why not. I'll take a look later if no one else figures it out.

@mavasani mavasani added help wanted The issue is up-for-grabs, and can be claimed by commenting False_Negative No diagnostic is reported for a problematic case labels Feb 17, 2021
@mavasani mavasani added this to the Unknown milestone Feb 17, 2021
@dotpaul dotpaul closed this as completed in 920d412 Mar 2, 2021
@jmarolf jmarolf modified the milestones: Unknown, .NET 6 Preview 3 Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category-Security DataFlow Enhancement False_Negative No diagnostic is reported for a problematic case help wanted The issue is up-for-grabs, and can be claimed by commenting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants