Skip to content

WebSocketWorker does not handle Throwable #1160

Closed
@Marcono1234

Description

@Marcono1234

Describe the bug
The WebSocketWorker does not catch thrown (non-Exception) Throwables; instead they cause the worker thread to die. Even though in general catching Throwable should be avoided it seems appropriate here, otherwise an adversary could exploit a vulnerability in the application, killing one worker thread after the other until no more workers are remaining, causing a denial of service.

Even though java.lang.Error is described as:

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch.

Some of the subclasses are thrown for non-serious problems, e.g. StackOverflowError could occur when malicious deeply nested data is deserialized.

To Reproduce
Steps to reproduce the behavior:

  1. Modify the onMessage(WebSocket, String) method of the example ChatServer to throw a java.lang.Error (simulating an error in the application code)
  2. Start the server
  3. Run the ExampleClient multiple times
    ❌ At some point all worker threads have died

Expected behavior
WebSocketWorker should catch Throwable and log it as fatal error, but it should not terminate due to it.

Alternatively shut down the complete server so whoever started it will notice that an error occurred (and can restart the server).

Environment

  • Version used: 1.5.3-SNAPSHOT (5f8d597)

Additional context
It appears in previous versions the following catch block for RuntimeExceptions would have caused a complete server shutdown, see #207:

} catch (RuntimeException e) {
handleFatal(ws, e);
}

However, in current versions that catch block cannot be reached (except for bugs caused by this library) since the called method doDecode catches and logs any thrown Exception.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions