Project

General

Profile

Actions
Copy link

Feature #7585

closed

Enhance logging of unexpected GET requests on Ajax sessions

Added by Bruce Toll over 4 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
Start date:
05/26/2020
Due date:
% Done:

0%

Estimated time:

Description

Once a Wt Ajax session has been established, the client no longer needs to send GET requests containing wtd query parameters.

Attached, for your review, is a patch that logs unexpected GET requests on Ajax sessions and returns a 403 status. It has been lightly tested on top of github master 4.3.0-26-gd18ca1b9, but would benefit from additional review and testing.

Files

Download all files
0001-Log-unexpected-GET-requests-on-Ajax-sessions.patch (2.1 KB) 0001-Log-unexpected-GET-requests-on-Ajax-sessions.patch Bruce Toll, 05/26/2020 01:28 AM
0002-Log-unexpected-GET-requests-on-Ajax-sessions.patch (2.7 KB) 0002-Log-unexpected-GET-requests-on-Ajax-sessions.patch Bruce Toll, 05/26/2020 09:53 PM
Actions
Copy link
 #1

Updated by Roel Standaert over 4 years ago

Hm, it's not quite right yet, since it makes it impossible to set reload-is-new-session to false. GET requests with an existing wtd should be tolerated in that case. I think if that configuration needs to be secured a bit better, then the Combined session tracking strategy can be used.

Also, I think the check is maybe a bit late? I see that some of the code before that already changes the state of the web session.

Actions
Copy link
 #2

Updated by Bruce Toll over 4 years ago

Thanks for the helpful feedback. I agree with you that the check is late. In an earlier attempt, I had placed the test in WebController::handleRequest before the web session was accessed, but it seemed like that approach required redundant work that belonged in WebSession. I will revisit. Perhaps I could add a const method in WebSession that helps determine whether a request should be rejected. I will also be sure to test with reload-is-new-session false...

Actions
Copy link
 #3

Updated by Bruce Toll over 4 years ago

Hi Roel,

I have attached a revised patch, relocating the check so that it occurs earlier in WebSession::handleRequest. I believe that if the check fails, it should now leave the session unaffected. I also moved up the definition of requestForResource so that it could be used in the new check and a couple of other locations.

I tested with reload-is-new-session false, as well as other configuration options, with and without JS. The flexibility of the library makes for a lot of testing combinations. Please review at your convenience. I'm happy to make additional changes, but if it's easier to just change yourself, feel free to do so. Thanks. ---Bruce

Actions
Copy link
 #4

Updated by Roel Standaert over 4 years ago

  • Status changed from New to Resolved

Thanks, Bruce.

I pushed it to GitHub. I also backported it to Wt 3.

Actions
Copy link
 #5

Updated by Roel Standaert over 4 years ago

  • Status changed from Resolved to Closed
Actions
Copy link

Also available in: Atom PDF