X Tutup
The Wayback Machine - https://web.archive.org/web/20240324153743/https://github.com/github/codeql/issues/4993
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 positive - Code Scanning - Java - when urlConnection.getInputStream() is not remote user input #4993

Open
JLLeitschuh opened this issue Jan 20, 2021 · 1 comment

Comments

@JLLeitschuh
Copy link
Contributor

Description of the false positive

The following is not valid remote user input.

public final class ResourceReader {

    public static String read(ClassLoader classLoader, String path, String fileDesc) {
        URL resource = classLoader.getResource(path);
        if (resource == null) {
            throw new IllegalStateException(String.format("Did not find resource '%s' on classpath.", path));
        }

        URLConnection urlConnection;
        try {
            urlConnection = resource.openConnection();
        } catch (IOException e) {
            throw new RuntimeException(String.format("Could not open connection for resource '%s'.", path), e);
        }

        InputStream inputStream;
        try {
            inputStream = urlConnection.getInputStream(); // CODEQL FLAGS THIS AS USER INPUT: But it isn't
        } catch (IOException e) {
            throw new RuntimeException(String.format("Could not get input stream of connection for resource '%s'.", path), e);
        }

        int length = urlConnection.getContentLength();
        if (length > 1024) {
            throw new IllegalStateException(String.format("'%s' is larger than 1 KiB.", fileDesc));
        }

        try (BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8), length)) {
            return reader.readLine();
        } catch (IOException e) {
            throw new RuntimeException(String.format("Error while reading input stream for resource '%s'.", path), e);
        }
        // ignore
    }

    private ResourceReader() {
    }

}

ClassLoader.getResource can't get an external resource.

    @Test
    void resourcesGet() throws IOException {
        URL resource = ResourcesTest.class.getClassLoader().getResource("https://google.com");
        assertNotNull(resource); // This test will fail as resource is null
    }

I think that if urlConnection.getInputStream() comes from a class loader, it shouldn't be considered a valid source.

@JLLeitschuh JLLeitschuh changed the title false positive - Code Scanning - Java false positive - Code Scanning - Java - urlConnection.getInputStream() not remote user input Jan 21, 2021
@JLLeitschuh JLLeitschuh changed the title false positive - Code Scanning - Java - urlConnection.getInputStream() not remote user input false positive - Code Scanning - Java - when urlConnection.getInputStream() is not remote user input Jan 21, 2021
@JLLeitschuh
Copy link
Contributor Author

This false positive, as well as other uses of getInputStream() where user input isn't involved, is so common that, at this point, if I see urlConnection.getInputStream() as the taint source, I don't look any further. I've found this "user-input" is over-classified as a user input. The scenarios where it's considered a "user-input" should be reduced in general.

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

No branches or pull requests

1 participant
X Tutup