1

Following security vulnerability has been reported on our application -

The call to readLine() at xyz.java line 119 might allow an attacker to crash the program or otherwise make it unavailable to legitimate users.

Code with vulnerability
Commented line reported -

BufferedReader reader = new BufferedReader(new InputStreamReader(
                        httpConnection.getInputStream()));
                String inputLine;
                StringBuffer okResponse = new StringBuffer();
                while ((inputLine = reader.readLine()) != null) { //readLine() on this line has been reported
                    okResponse.append(inputLine);
                }
                reader.close();
                return okResponse.toString();

The remediation for the same says -

Validate user input to ensure that it will not cause inappropriate resource utilization.

But, it is not clear what exactly can be validated. Any pointers?

Sandeepan Nath
  • 9,966
  • 17
  • 86
  • 144
  • [here](https://stackoverflow.com/questions/17084657/most-robust-way-of-reading-a-file-or-stream-using-java-to-prevent-dos-attacks/17142341#17142341) and [here](https://stackoverflow.com/questions/12412149/how-to-guard-against-resource-exhaustion-and-other-vulnerabilities); – LMC Jul 11 '19 at 17:51

1 Answers1

2

In theory an attacker could send you an unlimited amount of data via the httpConnection. Since you try to consume all of it, it might crash your application (OutOfMemory).

I´d assume you have a certain format and length for the okResponse in mind, so you would be better off checking that.


Make sure you read to read the answer from Subhas linked by Luis Muñoz
(Most Robust way of reading a file or stream using Java (to prevent DoS attacks)),
that has some more implementation details for reading content from a stream.


Another issue might be that the attack just keeps the connection open without sending any data. I`d assume there should be a timeout in place to cut the connection at some point, else the thread might be blocked forever.

Edit:
Since you code does not have it explicitly, you also should add a try ... finally ... block to make sure that the resources are properly closed.

second
  • 4,069
  • 2
  • 9
  • 24