spotbugs / discuss

SpotBugs mailing list
6 stars 1 forks source link

<Local> Filter Tag Doesn't Seem to Work #75

Closed kev22257 closed 5 years ago

kev22257 commented 5 years ago

I'm trying to create specific exclusions for SpotBugs (specifically for the security plugin) and when I try to specify a class, method and local variable the filter does not work. When I remove the Local part of the match the issue is correctly avoided.

    <Match>
        <Class name="com.a.b.dao.FileHandlingDao" />
        <Method name="getImage" />
        <Local name="imageUrl" /> <!-- This does not work -->
        <Bug pattern="URLCONNECTION_SSRF_FD" />
    </Match>

The documentation talks about Local, but there are no examples using it. Here is a snip of the program.

    public byte[] getImage(String id) {
        HttpURLConnection conn = null;
        InputStream response = null;

        try {
            URL imageUrl = new URL(new StringBuilder()
                    .append("https://myimagesite.org/img/")
                    .append(id).append(".png").toString());

I am trying to understand if something is broken or if I'm using the feature wrong/ misunderstanding its intent.

KengoTODA commented 5 years ago

Could you provide a mcve project? Especially we need:

kev22257 commented 5 years ago

Yes!

I think everything you need is in the pom.xml I included (I am using the SpotBugs Maven plugin) except for the JVM information:

java version "1.8.0_121" Java(TM) SE Runtime Environment (build 1.8.0_121-b13) Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode)

kev22257 commented 5 years ago

Also, I did fiddle with adding two different configurations to the Maven compiler plugin, but that didn't work. One was debug and debuglevel which did not change the behavior. One was compiler args (-g) which also did not change the behavior (should be the same as debug, debuglevel from what I understand).

<configuration>
    <source>1.8</source>
    <target>1.8</target>
    <debug>true</debug>
    <debuglevel>lines,vars,source</debuglevel>
</configuration>
KengoTODA commented 5 years ago

I think this error is detected in not local variable but a method execution initializing local variable:

[ERROR] Request header can easily be altered by the client [test.Test] At Test.java: [line 8] SERVLET_HEADER

In the generated report, we can see that no local variable is linked with this bug instance:

<BugInstance instanceOccurrenceNum='0' instanceHash='f8472e421c7704eee0b773c53682b52e' rank='15' abbrev='SECSH' category='SECURITY' priority='3' type='SERVLET_HEADER' instanceOccurrenceMax='0'>
  <ShortMessage>HTTP headers untrusted</ShortMessage>
  <LongMessage>Request header can easily be altered by the client</LongMessage>
  <Class classname='test.Test' primary='true'>
    <SourceLine classname='test.Test' start='5' end='20' sourcepath='test/Test.java' sourcefile='Test.java'>
      <Message>At Test.java:[lines 5-20]</Message>
    </SourceLine>
    <Message>In class test.Test</Message>
  </Class>
  <Method isStatic='true' classname='test.Test' signature='(Ljavax/servlet/http/HttpServletRequest;)Ljava/lang/String;' name='getIPAddress' primary='true'>
    <SourceLine endBytecode='145' classname='test.Test' start='7' end='14' sourcepath='test/Test.java' sourcefile='Test.java' startBytecode='0'></SourceLine>
    <Message>In method test.Test.getIPAddress(HttpServletRequest)</Message>
  </Method>
  <SourceLine endBytecode='10' classname='test.Test' start='8' end='8' sourcepath='test/Test.java' sourcefile='Test.java' startBytecode='10' primary='true'>
    <Message>At Test.java:[line 8]</Message>
  </SourceLine>
</BugInstance>

This is why <Local> doesn't work as your expectation. In this case, <Class>, <Method> and <Source> should work. We can use <Local> when local variable itself has a potential problem (e.g. local variable hides fields/variables in the external scope).

kev22257 commented 5 years ago

Interesting, so isn't useful for narrowing down to a specific line. I think that's unfortunate because if there was another bug introduced later in this method related to getHeader() then this exclusion could cause a bug to not be reported. Thanks for the clarification.