Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synchronous XMLHttpRequest deprecation? #137

Closed
jellisgwn opened this issue Aug 20, 2020 · 6 comments
Closed

Synchronous XMLHttpRequest deprecation? #137

jellisgwn opened this issue Aug 20, 2020 · 6 comments

Comments

@jellisgwn
Copy link

Chrome Version 84.0.4147.135 is reporting:

JavaScriptServlet:99 [Deprecation] Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.

in the hijackStandard() method:

	/** hook using standards based prototype **/
	function hijackStandard() {
		XMLHttpRequest.prototype._open = XMLHttpRequest.prototype.open;
		XMLHttpRequest.prototype.open = function(method, url, async, user, pass) {
			this.url = url;

			this._open.apply(this, arguments); \\ <-- here
		};

		XMLHttpRequest.prototype._send = XMLHttpRequest.prototype.send;
		XMLHttpRequest.prototype.send = function(data) {
			if(this.onsend != null) {
				this.onsend.apply(this, arguments);
			}

			this._send.apply(this, arguments);
		};
	}

Is this something that can be cleaned up?

@forgedhallpass
Copy link
Collaborator

I intend to include the fix for this in a larger refactoring that I am doing.

@jellisgwn
Copy link
Author

@forgedhallpass thanks for the reply.

Is it correct to assume that simply changing the request to be asynchronous would be a security problem?

@forgedhallpass
Copy link
Collaborator

Normally it shouldn't be, but I haven't had the time to test everything out yet. Besides setting the async flag to true, the responses must be handled in callbacks.

@forgedhallpass
Copy link
Collaborator

There is a race condition while requesting the page tokens asynchronously if some of the protected pages are referenced from IFRAMEs or IMG tags, that results in false-positive attack attempts.

@forgedhallpass
Copy link
Collaborator

Related to #51

@forgedhallpass
Copy link
Collaborator

Because of the above mentioned race condition, the async flag was made configurable in the new code-base. The test application was also updated with a new page dedicated to this special scenario.

The current approach of the OWASP CSRFGuard relies on JavaScript logic for injecting CSRF tokens into HTML elements or XHR requests. Forcing synchronous loading of the AJAX requests has been disabled, since they were deprecated due to their negative impact on the user experience. For this reason, protecting resources that would load before or in parallel with the JavaScript logic (e.g. references IFrames or IMG tags) is not possible.
In most cases this should not be a problem, because usually GET requests should not facilitate state-changing operations. If this last condition cannot be fulfilled (e.g. for legacy applications), backwards compatibility can be achieved by enabling the "forceSynchronousAjax" property within the configurations, until there is browser support for it.

You can find the new release candidate under releases under the official repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants