ModScurity blocks requests with enter

kamuffe commented 2 years ago

Description of the bug

Hi Vaadin Team.

When creating a Button with a ClickShortcut, the request to the server is intercepted by the ModSecurity WAF.

Setup: Client Browser <-> Apache (with ModeSecurity) as Proxy <-> Vaadin Server Application

Current Behavior: Vaddin Client Side sends a request with the following content to the server.

{
  "csrfToken": "atoken",
  "rpc": [{
      "type": "event",
      "node": 1,
      "event": "keydown",
      "data": {
        "event.shiftKey": false,
        "event.metaKey": false,
        "event.code": "Enter",
        "event.key": "Enter",
        "event.isComposing": false,
        "(['Enter'].indexOf(event.code) !== -1 || ['Enter'].indexOf(event.key) !== -1) && !event.getModifierState('Shift') && !event.getModifierState('Control') && !event.getModifierState('Alt') && !event.getModifierState('AltGraph') && !event.getModifierState('Meta') && (event.stopPropagation() || true)": true,
        "event.ctrlKey": false,
        "event.repeat": false,
        "event.location": 0,
        "event.altKey": false
      }
    }
  ],
  "syncId": 62,
  "clientId": 47
}

Because of the JavaScript in the json, a ModSecurity rule is triggered, and the request is blocked.

ModSecurity Error (snipped): "Warning. Pattern match \"(?i)[\\\"\\\\'][ ]*(?:[^a-z0-9~_:\\\\' ]|in).+?[.].+?=\" at ARGS_NAMES:rpc.rpc.data.(['Enter'].indexOf(event.code) !== -1 || ['Enter'].indexOf(event.key) !== -1) && !event.getModifierState('Shift') && !event.getModifierState('Control') && !event.getModifierState('Alt') && !event.getModifierState('AltGraph') && !event.getModifierState('Meta') && (event.stopPropagation() || true). [file \"/usr/local/apache2/conf/coreruleset-3.3.2/rules/REQUEST-941-APPLICATION-ATTACK-XSS.conf\"] [line \"829\"] [id \"941340\"] [msg \"IE XSS Filters - Attack Detected\"] [data \"Matched Data: '].indexOf(event.code) != found within ARGS_NAMES:rpc.rpc.data.(['Enter'].indexOf(event.code) !== -1 || ['Enter'].indexOf(event.key) !== -1) && !event.getModifierState('Shift') && !event.getModifierState('Control') && !event.getModifierState('Alt') && !event.getModifierState('AltGraph') && !event.getModifierState('Meta') && (event.stopPropagation() || true): rpc.rpc.data.(['Enter'].indexOf(event.code) !== -1 || ['Enter'].indexOf(event",

Best, Kamuffe

Expected behavior

No javascript is sent over the wire.
ClickShortcuts are working and are not intercepted by the WAF when setup includes a WAF

Minimal reproducible example

STR: Create a Vaadin application with a primary button with a click shortcut like:

Button button = new Button( caption );
button.addClickListener( clickListener );
button.addClickShortcut( Key.ENTER );

Setup Apache as Proxy with ModSecurity forwarding all requests to the Vaadin application.
Access the application through the Apache proxy by any browser.
Hit "Enter" in the keyboard to trigger the request.

Versions

Vaadin / Flow version: 14.8.6
Java version: 11
OS version: CentOS 7
Browser version (if applicable): does not matter
Application Server (if applicable): does not matter
IDE (if applicable): does not matter
ModSecurity CoreRuleset 3.3.2

Legioth commented 2 years ago

What you describe sounds like another case for https://vaadin.com/labs/csp. While the mechanism is slightly different (CSP vs ModSecurity), the core of it is that all the JS that needs to be run should be known at compile time rather than assembled by the server.

mshabarov commented 6 months ago

Seems like the long JS string is used as an ID of the shortcut triggered on the client. The solution might be to make a map of shortcuts on the server (and somehow synchronised with the client) and then the client can only send an ID of the shortcut. Needs a design.

Also, this looks to be more as a major issue as it prevents using server-side API for shortcuts with proxies.

Legioth commented 6 months ago

As a simpler workaround, we could maybe just create a built-in JS function for that particular case so that the whole expression wouldn't have to be sent back and forth. That would have the additional benefit of reducing payload sizes regardless of this particular issue.

vaadin / flow