Add BoundedBufferedReader class to prevent readLine DoS

[meg23]

From seantmalone on November 11, 2010 12:08:23

I recently did a code review for a project that was using the readLine() function of java.io.BufferedReader to read a user-controlled file. The security issue here is that, even if the number of lines to be read is limited, an attacker can still cause an OutOfMemoryError exception by providing a large file with no newline characters. The readLine function will just keep going until it runs out of memory, creating a denial of service.

My team did some research into this issue, but the only thing we found was an old bug report that had been closed with the status of "Will Not Fix." The suggested workaround was buggy and inconvenient, so I wrote the BoundedBufferedReader class to extend BufferedReader and add the capability to limit both line length and the number of lines read. This class can be used in much the same way as the original BufferedReader class.

I would like to offer this class as a contribution to the ESAPI project, as I believe there are many developers who could benefit from using this code. I welcome any feedback you have, and would be happy to work with the ESAPI team to make whatever changes are necessary to integrate this class into ESAPI.

I have attached the following files:

• BoundedBufferedReader.java is the main class

• ExampleUsage.java processes the test case file using the BoundedBufferedReader. Usage: "java ExampleUsage [maxLines] [maxLineLength]"

  Both of these can be compiled with a simple "javac *.java")

• TestCase.txt is the test case file, which includes lines of various lengths, multiple types of newline chars, and some NULL chars.

• TestCase_output_Ref.text -- output of "java ExampleUsage"

• TestCase_output_Ref_20_20.text -- output of "java ExampleUsage 20 20"

• TestCase_output_Ref_1000_10.text -- output of "java ExampleUsage 1000 10"

Please let me know if there is anything else you need.

Attachment: TestCase.txt TestCase_output_Ref.txt TestCase_output_Ref_20_20.txt TestCase_output_Ref_1000_10.txt

Original issue: http://code.google.com/p/owasp-esapi-java/issues/detail?id=183

[meg23]

From chrisisbeef on November 20, 2010 13:08:05

Status: Accepted
Labels: -Type-Defect Type-Enhancement Milestone-Release2.1 Component-Other

[meg23]

From santoste...@gmail.com on January 10, 2012 07:24:25

Hi there,
wonderful work there.. was just experimenting with it and observed that it does not read the last line of the file. in my case, i had just given number of characters alone as a constraint ( number of lines doesn matter). lines 57-59 in BoundedBufferedReader.java need some tweaking to return the last line and return null thereafter(in the next iteration of readline).

Thanks again for the good work. It saved me a lot of time.

Cheers.

[meg23]

From seantmalone on January 10, 2012 13:32:15

Thanks for the bug report, and I'm glad you found the class useful. I've updated the class to resolve the issue you described. I have put the project on github, so the latest version can be found at https://github.com/seantmalone/BoundedBufferedReader .

Please feel free to file an issue on github if there are any other problems, and I'll do my best to address them.

[shuklasan78]

How do i use the above concept for socket connection that reads data from input stream.
My code is
try (Socket httpSocket = server.accept();
BufferedReader reader = new BufferedReader(new InputStreamReader(httpSocket.getInputStream(),"UTF8"),2048);
BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(httpSocket.getOutputStream(),"UTF8"),2048);) {
httpSocket.setSoTimeout(600);
reader.ready();
line = reader.readLine();
isPost = line.startsWith("post");
int contentLength = 0;
while (!(line = reader.readLine()).equals("")) {
if (isPost) {
final String contentHeader = "Content-Length: ";
if (line.startsWith(contentHeader)) {
contentLength = Integer.parseInt(line.substring(contentHeader.length()));
}
}
}
I get denial of service and null pointer