Stray Thread hogging CPU after penetration testing with qualys

[DJGummikuh]

We have witnessed an odd behavior of membrane at one of our customers after repeated abuse by qualys.

It appears that with a chance of roughly 25%, their test creates a thread in membrane that does never end and eats up one CPU core. After 8 recorded tests, we had 2 stray threads with the following Stacktrace:

   java.lang.Thread.State: RUNNABLE
	at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
	at java.io.BufferedInputStream.skip(BufferedInputStream.java:380)
	- locked <0x00000000e1cff1a8> (a java.io.BufferedInputStream)
	at com.predic8.membrane.core.http.Body.discard(Body.java:90)
	at com.predic8.membrane.core.http.Message.discardBody(Message.java:77)
	at com.predic8.membrane.core.transport.http.HttpServerHandler.removeBodyFromBuffer(HttpServerHandler.java:269)
	at com.predic8.membrane.core.transport.http.HttpServerHandler.process(HttpServerHandler.java:241)
	at com.predic8.membrane.core.transport.http.HttpServerHandler.run(HttpServerHandler.java:119)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

   Locked ownable synchronizers:
	- <0x00000000e102cbc0> (a java.util.concurrent.ThreadPoolExecutor$Worker)

We assume that the culprit is the following section in the Body class (in the discard() method)

		while (toSkip > 0) {
			toSkip -= inputStream.skip(toSkip);
		}

Which will never exit in case .skip continuously returns "0" (which is clearly possible, regarding both the javadoc of InputStream.skip and the implementation of BufferedInputStream.skip and .fill)

Would it make any sense whatsoever to continue reading once .skip returned 0 or could we define this as a secondary exit parameter? Why are you pro-actively draining the inputstream anyways and don't just close it and let the operating system handle it?

Added: After dumping the heap and analyzing it, we found the following content in the BufferedInputStream's Buffer which clearly shows the involvement of Qualys:

POST /rest/json/login HTTP/1.1..Connection: Keep-Alive..Content-Type: application/json..X-Requested-With: XMLHttpRequest..Content-Length: 36..Host: 10.185.85.229..Qualys-Scan: VM....{"user":"admin","password":"admin"}ugins/kish-guest-posting/uploadify/scripts/uploadify.css HTTP/1.1..Host: hostname:9000..Connection: Keep-Alive..Qualys-Scan: VM....GET /Documentation.html HTTP/1.1..Host: hostname:9000..Connection: Keep-Alive..Qualys-Scan: VM....GET /wp-content/plugins/wp-property/third-party/uploadify/uploadify.css HTTP/1.1..Host: hostname:9000..Connection: Keep-Alive..Qualys-Scan: VM....GET /manager/templates/default/header.tpl HTTP/1.1..Host: hostname:9000..Connection: Keep-Alive..Qualys-Scan: VM....GET /console/login/LoginForm.jsp HTTP/1.1..Host: hostname:9000..Connection: Keep-Alive..Qualys-Scan: VM....GET /wp-content/plugins/easy-post-types/classes/custom-image/media.php?ref=ref"</script><iframe+onload%3Dalert(%2FQUALYSXSSTEST%2F)> H

[rrayst]

Interesting, thanks for the report!!

Why are you pro-actively draining the inputstream anyways and don't just close it and let
the operating system handle it?

At this point in the code, there could be further HTTP requests coming in over the same TCP connection. As the first request has not yet been fully read, because the body was never used (and neither read yet), it has to be removed (=skipped) from the incoming connection to - possibly - get to the second request.

If skip keeps returning 0, something should be done about this.

[DJGummikuh]

Ah I see that makes sense. However we assume that these threads were burning cpu cycles since for about 2 weeks so something was not working right. I do have no idea however how we could reproduce this behavior probably as apparently not even the qualys abuse reproducible caused this misbehavior

[DJGummikuh]

Just for clarification, are you planning on taking a look at it? I'd try to fix it but I fear I'd break it in some unintuitive way so I'm a bit hesitant :)

[DJGummikuh]

bump

[DJGummikuh]

bump

[rrayst]

Feel free to reopen, if this is still an issue.