akhikhl / gretty

Advanced gradle plugin for running web-apps on jetty and tomcat.
MIT License
656 stars 174 forks source link

OutOfMemory on invalid data sent to service port #169

Open jest opened 9 years ago

jest commented 9 years ago

When non-service data is sent to the service port (e.g. with some misconfigured client of other software), Gretty allocates new data indefinitely, which results in OOM.

The reason is the optimistic waiting for <<EOF>> string in the command sent. However, if some data is sent to the socket and the socket is closed by the connecting party, the following lines result in assigning null to line and appending "null" String value to the StringBuilder:

while(true) {
  String line = reader.readLine()
  if(line == '<<EOF>>')
    break
  data << line
}

The solution would be to check for line == null and terminate early with e.g. empty command.

akhikhl commented 9 years ago

Hi, The problem should be fixed in Gretty 1.2.4. Now Gretty automatically chooses unused TCP ports for service data exchange, so there should be no such conflict. Could you, please, verify that it works?

jest commented 9 years ago

The problem is not a port number clash, but not terminating reading lines in case of connection being terminated before <<EOF>>.

akhikhl commented 9 years ago

which app writes to service port?

jest commented 9 years ago

@akhikhl, this issue is not about missing functionality, but about potential — intentional or not — denial of service.

The random port is definitely going to limit the threat, but the correct fix would be additionally to leave the loop on stream EOF:

  if(line == '<<EOF>>' || line == null)
    break

Without this you still can imagine an attacker doing port scan and crashing the VM with the web application. IMO that's a security issue.

akhikhl commented 9 years ago

Thanks for very interesting and deep observation. I didn't think of it. I'll see to improving the service protocol.