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

False negative about the rule SQL_BAD_RESULTSET_ACCESS in java #2137

Open
vanguard-1024 opened this issue Aug 7, 2022 · 3 comments
Open

Comments

@vanguard-1024
Copy link

Hi, we are doing research in testing static analyzer. Our approach found a false negative about the rule SQL_BAD_RESULTSET_ACCESS, SpotBugs should report a warning in line 6 because this line invokes getString with parameter 0.

import java.sql.ResultSet;
import java.sql.SQLException;
public class Test {
    public int a = 0;
    void bug1(ResultSet any) throws SQLException {
        any.getString(a);  // should report a warning in this line, but no warning
    }
}

However, the following two similar code examples can be detected:

Example 1:

public class Test {
    void bug1(ResultSet any) throws SQLException {
        int a = 0;
        any.getString(a);  // can report a warning in this line
    }
}

Example 2:

public class Test {
    int a = 0;
    void bug1(ResultSet any) throws SQLException {
        any.getString(a);  // can report a warning in this line
    }
}

Based on the above analysis results, I think this is a false negative. Thanks for your consideration.

SpotBugs version: 4.7.1

@welcome
Copy link

welcome bot commented Aug 7, 2022

Thanks for opening your first issue here! 😃
Please check our contributing guideline. Especially when you report a problem, make sure you share a Minimal, Complete, and Verifiable example to reproduce it in this issue.

@vanguard-1024
Copy link
Author

We also find another false negative about this rule, SpotBugs should report a waning at line 8:

import java.sql.ResultSet;
import java.sql.SQLException;
public class C {
    int foo() {
        return 0;
    }
    void bug1(ResultSet any) throws SQLException {
        any.getString(foo());  // should report a warning
    }
}

@baloghadamsoftware
Copy link
Contributor

Hello! Thank you for your issue report. Actually, you report two issues here.

The first one I am sure is not a false negative, because a is public but it is not final. This means that anyone, even from outside the package can change its value. Thus, at the time of calling bug1() we cannot know anything about its actual value.

The second case is indeed a false negative. To detect such bugs we need to introduce function summaries. This is something that could (and should) be done in the future. Hower, functions returning a constant are a very corner case, thus function summaries should cover a wider case. Maybe, if we could keep track somehow the possible value ranges a function returns we could record them as the summary of the function. (See #2073, it is a first step in that direction.) Then the summary of your function foo() would be a single range [0..0] and we could detect the bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants