Closed
Bug 1172233
Opened 10 years ago
Closed 9 years ago
Allow web packaged apps to be served without a mimetype
Categories
(Core :: Networking, defect)
Tracking
()
RESOLVED
FIXED
mozilla42
People
(Reporter: valentin, Assigned: valentin)
References
Details
Attachments
(3 files, 1 obsolete file)
|
5.10 KB,
patch
|
Details | Diff | Splinter Review | |
|
2.84 KB,
patch
|
Details | Diff | Splinter Review | |
|
16.25 KB,
patch
|
mcmanus
:
review+
|
Details | Diff | Splinter Review |
sicking: if the package was loaded without a "correct" mimetype, then serve all the resources in the package without a mimetype
sicking: i.e. we could still serve any resource, just don't provide a mimetype
sicking: that way gecko won't interpret a HTML resource as HTML. And won't run script in it
Valentine: I think it's fine if content inside a package ends up getting a sniffed mimetype. All we need to ensure is that if the package was served without the correct mimetype, that we for the resources inside the package, act as if the *server* didn't provide a mimetype.
If parts higher up in the stack then end up defaulting to, or sniffing, a mimetype, then that's fine.
The reason that's fine is that the same thing would happen if a the server served a non-packaged resource with the same mimetype.
However we should make sure that any headers which affect mime types and which are served with the http response are also added to the response to any resource inside the package. Once/if we implement the "x-content-type-options" header, that means that we should forward that header.
|
Assignee | |
Comment 2•9 years ago
|
||
Attachment #8624978 -
Flags: review?(mcmanus)
|
|
Assignee | |
Updated•9 years ago
|
Assignee: nobody → valentin.gosu
Status: NEW → ASSIGNED
|
|
Assignee | |
Comment 3•9 years ago
|
||
Attachment #8625388 -
Flags: review?(mcmanus)
|
||
Comment 4•9 years ago
|
||
Comment on attachment 8625388 [details] [diff] [review]
Test for web packaged apps served with a wrong content-type
Review of attachment 8625388 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/test/mochitests/test_web_packaged_app.html
@@ +48,5 @@
> + iframe.src = "web_packaged_app.sjs!//index.html";
> +}
> +
> +function check_mimetype() {
> + ok(iframe.contentWindow.document.body.innerHTML.includes("Web Packaged App Index"), "This is the right file");
I suggest we add a test to verify that scripts can not execute for packages with missing mimetypes.
|
||
Updated•9 years ago
|
Attachment #8624978 -
Flags: review?(mcmanus) → review+
|
|
||
Comment 5•9 years ago
|
||
Comment on attachment 8625388 [details] [diff] [review]
Test for web packaged apps served with a wrong content-type
Review of attachment 8625388 [details] [diff] [review]:
-----------------------------------------------------------------
pls add the case :freddyb suggests - good idea.
Attachment #8625388 -
Flags: review?(mcmanus)
|
|
||
Comment 6•9 years ago
|
||
Paul, you wanted a reminder/needinfo about this bug.
Flags: needinfo?(ptheriault)
|
|
Assignee | |
Comment 7•9 years ago
|
||
(In reply to Frederik Braun [:freddyb] from comment #4)
> I suggest we add a test to verify that scripts can not execute for packages
> with missing mimetypes.
Just to double check here, according to your previous comments, resources from packages without the correct
mime-type should be served without a "Content-Type" header. In this case, accessing a index.html inside a package, would still run the JS, as it will sniff out the content-type, right? (I checked, and the same happens for a regular file without a mime-type).
Flags: needinfo?(jonas)
The easiest way to test this is by creating a mochitest which contains a
<script>
wasCalled = false;
function callback() {
wasCalled = true;
}
function loadHandler() {
is(wasCalled, false);
}
</script>
<iframe src="package.pak!//index.html"></iframe>
Then make the package contain an index.html like:
<html><head><script>parent.callback();</script>
In this case gecko will *not* sniff a text/html mimetype, and should *not* execute the script. I.e. if necko returns a response which does not contain a a content-type header. I suspect gecko will just render the contents as text/plain or possibly display a save-as dialog.
The same thing will happen if the <iframe> points to a plain index.html page for which the server does not send a mimetype. Gecko should *not* sniff the mimetype and should *not* execute the script.
The easiest way to test this is by loading
data:text/html,<iframe src="data:,<html><script>console.log('hi')</script>iframe contents"></iframe>outer contents
and
data:text/html,<iframe src="data:text/html,<html><script>console.log('hi')</script>iframe contents"></iframe>outer contents
The former will display the iframe contents as text/plain and not run the script. The latter will parse the iframe contents as HTML and display 'hi' in the console.
Flags: needinfo?(jonas)
In the testcase above package.pak would be served without a mimetype, or with the wrong mimetype.
If package.pak is served with "all the right" mimetypes, both for the package and for index.html, then the script should indeed run.
|
|
Assignee | |
Comment 10•9 years ago
|
||
While the data: example worked as expected, the attached test case shows that we still run JS for a html file without a mimetype.
As far as the packaged app format is concerned, it behaves in exactly the same way.
Unless the test is wrong, this is the expected behaviour (I also tried this testcase in Chrome, and it works the same).
Attachment #8629688 -
Flags: feedback?(jonas)
|
|
||
Comment 11•9 years ago
|
||
I should be more explicit about what I want to avoid:
On a web page with an XSS vulnerability, an attacker could inject text (boundary, headers, ..) that make the returned resource look like a package. This could result in the browser caching a package that is not located at the (vulnerable) origin.
Summary:
When the package isn't sent with the correct Content-Type we should send "application/octet-stream" as Content-Type for the resources inside the package.
Details:
I had a chat with bz and it appears that my proposal was bad.
Let me step back a bit and first describe what we're trying to accomplish and what the constraints are.
My goal is to enable a web developer to package up resources, like scripts and images, where browsers already don't care about the mimetype. And then serve that package without worrying about the mimetype that the package is served with.
To put it in other words, since browsers don't normally care what mimetype an image file is served with, a web developer should be able to load a image from inside a package without caring with mimetype the package is served with.
However, there are a lot of websites out there that allow uploading user files and then host those files on the same domain as sensitive data. For example that allow the user to upload an avatar image file, or upload content which other users can then download. In order to prevent an attacker from XSSing that website by uploading a HTML/SVG/XHTML file containing evil script, the website currently has to take "certain steps".
We need to make sure that those "certain steps" also protect the website from XSS in case a package is uploaded. I.e. websites that are currently safe, should remain safe.
Generally speaking, the only safe "steps" that the website can take is to ensure that it uses a Content-Type which won't be parsed as HTML, SVG, XHTML or anything else that can contain script.
(I don't doubt that there are sites out there that prevent script execution by trying to look in the file contents to see if the contents might cause script execution, but I'm willing to bet money on that those can be worked around 100% of the time. So I don't consider those as safe "steps").
I erroneously thought that simply sending no Content-Type would cause browsers to not treat the content as anything potentially script-running. Especially not as HTML/SVG/XHTML. However it turns out that sending no Content-Type means "please sniff this content".
This definitely means that sending no Content-Type is not a safe thing for us to do when we get a package with the wrong Content-Type.
What we instead should do is to send "application/octet-stream" as Content-Type. That should give us the requested behavior in comment 8.
We should also think about what we want to do if a package is received with no Content-Type. It's probably not terribly important what we do here since sending no Content-Type is pretty rare and usually requires explicit action from the author. So for now we should probably keep things simple and treat no Content-Type the same as sending a wrong Content-Type.
|
|
||
Comment 13•9 years ago
|
||
The "certain step" most likely involves setting content-disposition. We should honor that as well.
That's a good point. We should also forward any content-disposition: attachment header from the package to each resource.
|
|
Assignee | |
Comment 15•9 years ago
|
||
Resubmitting for review following comment 12. Resources in packages with the wrong mimetype will now be served with the application/octet-stream content-type. Content-disposition will be handled in bug 1181137 along with other headers.
The test case now works (no longer runs the JS), but I'm having trouble interacting with the download pop-up. Still working on that.
Attachment #8633190 -
Flags: review?(mcmanus)
|
|
Assignee | |
Updated•9 years ago
|
Attachment #8624978 -
Attachment is obsolete: true
|
|
||
Comment 16•9 years ago
|
||
Comment on attachment 8633190 [details] [diff] [review]
Allow web packaged apps to be served without a mimetype
Review of attachment 8633190 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/streamconv/converters/nsMultiMixedConv.cpp
@@ +1090,5 @@
> mContentType = headerVal;
> +
> + // If the HTTP channel doesn't have an application/package
> + // content type we still want to serve the resource, but with
> + // no content type header, so we prevent execution of
comment still says "no content type header"
Attachment #8633190 -
Flags: review?(mcmanus) → review+
|
||
Comment 17•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Attachment #8629688 -
Flags: feedback?(jonas)
|
||
Updated•9 years ago
|
Flags: needinfo?(ptheriault)
You need to log in
before you can comment on or make changes to this bug.
Description
•