Same-origin checking for XForms submission

RESOLVED FIXED in mozilla1.8beta1

Status

Core Graveyard
XForms
RESOLVED FIXED
14 years ago
2 years ago

People

(Reporter: Darin Fisher, Assigned: Darin Fisher)

Tracking

Trunk
mozilla1.8beta1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

9.42 KB, patch
Brian Ryner (not reading)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

14 years ago
Same-origin checking for XForms submission

Because XForms submission enables the free form construction of XML data to be
submitted as part of a HTTP request, we should take care to limit where those
requests may be sent.  Moreover, since XForms enables the response to be seen by
the form (via replace="instance"), we should take care to limit where those
responses may come from.

In other words, it seems that XForms submission should have a security policy
that is similar to document.load or XMLHttpRequest.  However, since XForms
submission can be triggered without the use of JavaScript, we cannot necessarily
rely on having a JS context to base our security decisions on.  That's
unfortunate, but it shouldn't be a major problem.

Patch coming up...
(Assignee)

Comment 1

14 years ago
Created attachment 166864 [details] [diff] [review]
v1 patch
(Assignee)

Comment 2

14 years ago
Comment on attachment 166864 [details] [diff] [review]
v1 patch

NOTE: this patch is impossible to override.  at some point we will want to
invent a way to allow the browser to be configured to enable certain sites or
domains to support cross-origin XForms submission.

Also, this patch only limits XML submission or replace="instance" ...
replace="all/none" + method="get/form-data-post" need not be restricted.
Attachment #166864 - Flags: superreview?(jst)
Attachment #166864 - Flags: review?(bryner)
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta
Comment on attachment 166864 [details] [diff] [review]
v1 patch

sr=jst
Attachment #166864 - Flags: superreview?(jst) → superreview+
Attachment #166864 - Flags: review?(bryner) → review+
(Assignee)

Comment 4

14 years ago
Comment on attachment 166864 [details] [diff] [review]
v1 patch

Spoke with jst, and we came up with a better solution.	We can use CheckConnect
with the JSContext associated with the script global of our document.  This is
defined even when there is no JS on the page.  Using CheckConnect is
preferrable since there is already a mechanism to override it using prefs.
Attachment #166864 - Attachment is obsolete: true
(Assignee)

Comment 5

14 years ago
Actually, CheckConnect is not what we want.  More in a bit...
(Assignee)

Comment 6

14 years ago
Created attachment 167459 [details] [diff] [review]
v2 patch

Similar patch, but includes calls to nsIPermissionManager::TestPermission so
that the same-origin restriction can be disabled per-host.  NOTE: this does not
allow us to disable same-origin checking for protocols that do not have a host
(e.g., file:///).
(Assignee)

Comment 7

14 years ago
The v2 patch also fixes two unrelated problems that I observed:

 (1) We were not handling relative URIs specified as the "src" of a <instance> 
     element.

 (2) nsXFormsSetValueElement needs to null check the result of an xpath 
     evaluation.
(Assignee)

Updated

14 years ago
Attachment #167459 - Flags: review?(bryner)
Attachment #167459 - Flags: review?(bryner) → review+
(Assignee)

Comment 8

14 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.