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

AjaxProxy Enhancement #731

Merged
merged 1 commit into from
Apr 25, 2016
Merged

Conversation

lbane
Copy link
Contributor

@lbane lbane commented Apr 14, 2016

There seems to be a confusion between the binding names "AjaxBridge" and "JSONRPCBridge" in AjaxProxy. Both binding names are used, but I believe they should mean the same object. The patch changes AjaxProxy to accept both names.

The patch also provides some enhancements to the documentation, i.e. a warning to never omit the proxy object.

@darkv darkv merged commit 06b1ad3 into wocommunity:master Apr 25, 2016
@paulhoadley
Copy link
Contributor

Hi Ralf,

I've just started seeing some output from your log warning:

log.warn("No proxy binding given, so using parent component. This is probably a very bad idea.");

I tracked it down to AjaxFlexibleFileUpload, which I'm using from ERAttachmentFlexibleUpload. I notice that AjaxFlexibleFileUpload certainly doesn't set the proxy binding:

AjaxProxy : AjaxProxy {
  name = ajaxProxyName;
  proxyName = "wopage";
}

How dangerous is this? The component certainly still works, but is this exposing a significant security risk, and if so can we fix it in AjaxFlexibleFileUpload?

@lbane
Copy link
Contributor Author

lbane commented Jun 17, 2016

AjaxProxy currently publishes every public method of the object on the client side. For components this means for example WOComponent.valueForKeyPath and WOComponent.takeValueForKeyPath are directly callable by the client. A malicious user may call valueForKeyPath("application.terminate") or every other path reachable by session or application. I call this a serious issue.

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

Successfully merging this pull request may close these issues.

3 participants