Bug 1297186 - Correctly propagate principals when getting the files list in DataTransfer, r=enndeakin
MozReview-Commit-ID: 8LRBekaOUJR
--- a/dom/events/DataTransfer.cpp
+++ b/dom/events/DataTransfer.cpp
@@ -278,34 +278,36 @@ DataTransfer::SetEffectAllowedInt(uint32
NS_IMETHODIMP
DataTransfer::GetMozUserCancelled(bool* aUserCancelled)
{
*aUserCancelled = MozUserCancelled();
return NS_OK;
}
-FileList*
+already_AddRefed<FileList>
DataTransfer::GetFiles(ErrorResult& aRv)
{
- return mItems->Files();
+ return mItems->Files(nsContentUtils::SubjectPrincipal());
}
NS_IMETHODIMP
DataTransfer::GetFiles(nsIDOMFileList** aFileList)
{
if (!aFileList) {
return NS_ERROR_FAILURE;
}
- ErrorResult rv;
- RefPtr<FileList> files = GetFiles(rv);
- if (NS_WARN_IF(rv.Failed())) {
- return rv.StealNSResult();
- }
+ // The XPCOM interface is only avaliable to system code, and thus we can
+ // assume the system principal. This is consistent with the previous behavour
+ // of this function, which also assumed the system principal.
+ //
+ // This code is also called from C++ code, which expects it to have a System
+ // Principal, and thus the SubjectPrincipal cannot be used.
+ RefPtr<FileList> files = mItems->Files(nsContentUtils::GetSystemPrincipal());
files.forget(aFileList);
return NS_OK;
}
already_AddRefed<DOMStringList>
DataTransfer::GetTypes(ErrorResult& aRv) const
{
@@ -692,32 +694,16 @@ DataTransfer::SetDataAtInternal(const ns
return NS_ERROR_DOM_INDEX_SIZE_ERR;
}
// Don't allow the custom type to be assigned.
if (aFormat.EqualsLiteral(kCustomTypesMime)) {
return NS_ERROR_TYPE_ERR;
}
- // Don't allow non-chrome to add non-string or file data. We'll block file
- // promises as well which are used internally for drags to the desktop.
- if (!nsContentUtils::IsSystemPrincipal(aSubjectPrincipal)) {
- if (aFormat.EqualsLiteral(kFilePromiseMime) ||
- aFormat.EqualsLiteral(kFileMime)) {
- return NS_ERROR_DOM_SECURITY_ERR;
- }
-
- uint16_t type;
- aData->GetDataType(&type);
- if (type == nsIDataType::VTYPE_INTERFACE ||
- type == nsIDataType::VTYPE_INTERFACE_IS) {
- return NS_ERROR_DOM_SECURITY_ERR;
- }
- }
-
return SetDataWithPrincipal(aFormat, aData, aIndex, aSubjectPrincipal);
}
void
DataTransfer::MozSetDataAt(JSContext* aCx, const nsAString& aFormat,
JS::Handle<JS::Value> aData,
uint32_t aIndex, ErrorResult& aRv)
{
@@ -831,17 +817,17 @@ DataTransfer::GetFilesAndDirectories(Err
return nullptr;
}
RefPtr<Promise> p = Promise::Create(global, aRv);
if (NS_WARN_IF(aRv.Failed())) {
return nullptr;
}
- RefPtr<FileList> files = mItems->Files();
+ RefPtr<FileList> files = mItems->Files(nsContentUtils::SubjectPrincipal());
if (NS_WARN_IF(!files)) {
return nullptr;
}
Sequence<RefPtr<File>> filesSeq;
files->ToSequence(filesSeq, aRv);
if (NS_WARN_IF(aRv.Failed())) {
return nullptr;
--- a/dom/events/DataTransfer.h
+++ b/dom/events/DataTransfer.h
@@ -141,17 +141,17 @@ public:
void GetData(const nsAString& aFormat, nsAString& aData, ErrorResult& aRv);
void SetData(const nsAString& aFormat, const nsAString& aData,
ErrorResult& aRv);
void ClearData(const mozilla::dom::Optional<nsAString>& aFormat,
mozilla::ErrorResult& aRv);
- FileList* GetFiles(mozilla::ErrorResult& aRv);
+ already_AddRefed<FileList> GetFiles(mozilla::ErrorResult& aRv);
already_AddRefed<Promise> GetFilesAndDirectories(ErrorResult& aRv);
already_AddRefed<Promise> GetFiles(bool aRecursiveFlag, ErrorResult& aRv);
void AddElement(Element& aElement, mozilla::ErrorResult& aRv);
--- a/dom/events/DataTransferItem.cpp
+++ b/dom/events/DataTransferItem.cpp
@@ -98,41 +98,46 @@ DataTransferItem::SetData(nsIVariant* aD
break;
}
}
mData = nullptr;
return;
}
- mKind = KIND_OTHER;
mData = aData;
+ mKind = KindFromData(mData);
+}
+/* static */ DataTransferItem::eKind
+DataTransferItem::KindFromData(nsIVariant* aData)
+{
nsCOMPtr<nsISupports> supports;
nsresult rv = aData->GetAsISupports(getter_AddRefs(supports));
if (NS_SUCCEEDED(rv) && supports) {
// Check if we have one of the supported file data formats
if (nsCOMPtr<nsIDOMBlob>(do_QueryInterface(supports)) ||
nsCOMPtr<BlobImpl>(do_QueryInterface(supports)) ||
nsCOMPtr<nsIFile>(do_QueryInterface(supports))) {
- mKind = KIND_FILE;
- return;
+ return KIND_FILE;
}
}
nsAutoString string;
// If we can't get the data type as a string, that means that the object
// should be considered to be of the "other" type. This is impossible
// through the APIs defined by the spec, but we provide extra Moz* APIs,
// which allow setting of non-string data. We determine whether we can
// consider it a string, by calling GetAsAString, and checking for success.
rv = aData->GetAsAString(string);
if (NS_SUCCEEDED(rv)) {
- mKind = KIND_STRING;
+ return KIND_STRING;
}
+
+ return KIND_OTHER;
}
void
DataTransferItem::FillInExternalData()
{
if (mData) {
return;
}
@@ -230,36 +235,44 @@ DataTransferItem::FillInExternalData()
NS_WARNING("Clipboard data provided by the OS does not match predicted kind");
}
#endif
}
already_AddRefed<File>
DataTransferItem::GetAsFile(ErrorResult& aRv)
{
+ return GetAsFileWithPrincipal(nsContentUtils::SubjectPrincipal(), aRv);
+}
+
+already_AddRefed<File>
+DataTransferItem::GetAsFileWithPrincipal(nsIPrincipal* aPrincipal, ErrorResult& aRv)
+{
if (mKind != KIND_FILE) {
return nullptr;
}
- nsCOMPtr<nsIVariant> data = Data(nsContentUtils::SubjectPrincipal(), aRv);
+ // This is done even if we have an mCachedFile, as it performs the necessary
+ // permissions checks to ensure that we are allowed to access this type.
+ nsCOMPtr<nsIVariant> data = Data(aPrincipal, aRv);
if (NS_WARN_IF(!data || aRv.Failed())) {
return nullptr;
}
- nsCOMPtr<nsISupports> supports;
- aRv = data->GetAsISupports(getter_AddRefs(supports));
- MOZ_ASSERT(!aRv.Failed() && supports,
- "File objects should be stored as nsISupports variants");
- if (aRv.Failed() || !supports) {
- return nullptr;
- }
-
// Generate the dom::File from the stored data, caching it so that the
// same object is returned in the future.
if (!mCachedFile) {
+ nsCOMPtr<nsISupports> supports;
+ aRv = data->GetAsISupports(getter_AddRefs(supports));
+ MOZ_ASSERT(!aRv.Failed() && supports,
+ "File objects should be stored as nsISupports variants");
+ if (aRv.Failed() || !supports) {
+ return nullptr;
+ }
+
if (nsCOMPtr<nsIDOMBlob> domBlob = do_QueryInterface(supports)) {
Blob* blob = static_cast<Blob*>(domBlob.get());
mCachedFile = blob->ToFile();
} else if (nsCOMPtr<BlobImpl> blobImpl = do_QueryInterface(supports)) {
MOZ_ASSERT(blobImpl->IsFile());
mCachedFile = File::Create(mDataTransfer, blobImpl);
} else if (nsCOMPtr<nsIFile> ifile = do_QueryInterface(supports)) {
mCachedFile = File::CreateFromFile(mDataTransfer, ifile);
@@ -270,17 +283,24 @@ DataTransferItem::GetAsFile(ErrorResult&
RefPtr<File> file = mCachedFile;
return file.forget();
}
already_AddRefed<FileSystemEntry>
DataTransferItem::GetAsEntry(ErrorResult& aRv)
{
- RefPtr<File> file = GetAsFile(aRv);
+ return GetAsEntryWithPrincipal(nsContentUtils::SubjectPrincipal(), aRv);
+}
+
+already_AddRefed<FileSystemEntry>
+DataTransferItem::GetAsEntryWithPrincipal(nsIPrincipal* aPrincipal,
+ ErrorResult& aRv)
+{
+ RefPtr<File> file = GetAsFileWithPrincipal(aPrincipal, aRv);
if (NS_WARN_IF(aRv.Failed()) || !file) {
return nullptr;
}
nsCOMPtr<nsIGlobalObject> global;
// This is annoying, but DataTransfer may have various things as parent.
nsCOMPtr<EventTarget> target =
do_QueryInterface(mDataTransfer->GetParentObject());
--- a/dom/events/DataTransferItem.h
+++ b/dom/events/DataTransferItem.h
@@ -43,17 +43,20 @@ public:
, mDataTransfer(aDataTransfer)
{
MOZ_ASSERT(mDataTransfer, "Must be associated with a DataTransfer");
}
already_AddRefed<DataTransferItem> Clone(DataTransfer* aDataTransfer) const;
virtual JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
+
+ // NOTE: This accesses the subject principal, and should not be called from C++
void GetAsString(FunctionStringCallback* aCallback, ErrorResult& aRv);
+
void GetKind(nsAString& aKind) const
{
switch (mKind) {
case KIND_FILE:
aKind = NS_LITERAL_STRING("file");
return;
case KIND_STRING:
aKind = NS_LITERAL_STRING("string");
@@ -74,19 +77,25 @@ public:
{
return mKind;
}
void SetKind(eKind aKind)
{
mKind = aKind;
}
+ // NOTE: This accesses the subject principal, and should not be called from C++
already_AddRefed<File> GetAsFile(ErrorResult& aRv);
+ already_AddRefed<File> GetAsFileWithPrincipal(nsIPrincipal* aPrincipal,
+ ErrorResult& aRv);
+ // NOTE: This accesses the subject principal, and should not be called from C++
already_AddRefed<FileSystemEntry> GetAsEntry(ErrorResult& aRv);
+ already_AddRefed<FileSystemEntry> GetAsEntryWithPrincipal(nsIPrincipal* aPrincipal,
+ ErrorResult& aRv);
DataTransfer* GetParentObject() const
{
return mDataTransfer;
}
nsIPrincipal* Principal() const
{
@@ -115,16 +124,18 @@ public:
{
return mChromeOnly;
}
void SetChromeOnly(bool aChromeOnly)
{
mChromeOnly = aChromeOnly;
}
+ static eKind KindFromData(nsIVariant* aData);
+
private:
~DataTransferItem() {}
already_AddRefed<File> CreateFileFromInputStream(nsIInputStream* aStream);
// The index in the 2d mIndexedItems array
uint32_t mIndex;
bool mChromeOnly;
--- a/dom/events/DataTransferItemList.cpp
+++ b/dom/events/DataTransferItemList.cpp
@@ -195,25 +195,58 @@ DataTransferItemList::Add(File& aData, E
if (NS_WARN_IF(aRv.Failed())) {
return nullptr;
}
MOZ_ASSERT(item->Kind() == DataTransferItem::KIND_FILE);
return item;
}
-FileList*
-DataTransferItemList::Files()
+already_AddRefed<FileList>
+DataTransferItemList::Files(nsIPrincipal* aPrincipal)
{
+ // The DataTransfer can hold data with varying principals, coming from
+ // different windows. This means that permissions checks need to be made when
+ // accessing data from the DataTransfer. With the accessor methods, this is
+ // checked by DataTransferItem::Data(), however with files, we keep a cached
+ // live copy of the files list for spec compliance.
+ //
+ // A DataTransfer is only exposed to one webpage, and chrome code. The chrome
+ // code should be able to see all files on the DataTransfer, while the webpage
+ // should only be able to see the files it can see. As chrome code doesn't
+ // need as strict spec compliance as web visible code, we generate a new
+ // FileList object every time you access the Files list from chrome code, but
+ // re-use the cached one when accessing from non-chrome code.
+ //
+ // It is not legal to expose an identical DataTransfer object is to multiple
+ // different principals without using the `Clone` method or similar to copy it
+ // first. If that happens, this method will assert, and return nullptr in
+ // release builds. If this functionality is required in the future, a more
+ // advanced caching mechanism for the FileList objects will be required.
+ RefPtr<FileList> files;
+ if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
+ files = new FileList(static_cast<nsIDOMDataTransfer*>(mDataTransfer));
+ GenerateFiles(files, aPrincipal);
+ return files.forget();
+ }
+
if (!mFiles) {
mFiles = new FileList(static_cast<nsIDOMDataTransfer*>(mDataTransfer));
+ mFilesPrincipal = aPrincipal;
RegenerateFiles();
}
- return mFiles;
+ if (!aPrincipal->Subsumes(mFilesPrincipal)) {
+ MOZ_ASSERT(false, "This DataTransfer should only be accessed by the system "
+ "and a single principal");
+ return nullptr;
+ }
+
+ files = mFiles;
+ return files.forget();
}
void
DataTransferItemList::MozRemoveByTypeAt(const nsAString& aType,
uint32_t aIndex,
ErrorResult& aRv)
{
if (NS_WARN_IF(mDataTransfer->IsReadOnly() ||
@@ -278,16 +311,32 @@ already_AddRefed<DataTransferItem>
DataTransferItemList::SetDataWithPrincipal(const nsAString& aType,
nsIVariant* aData,
uint32_t aIndex,
nsIPrincipal* aPrincipal,
bool aInsertOnly,
bool aHidden,
ErrorResult& aRv)
{
+ if (!nsContentUtils::IsSystemPrincipal(aPrincipal)) {
+ DataTransferItem::eKind kind = DataTransferItem::KindFromData(aData);
+ if (kind == DataTransferItem::KIND_OTHER) {
+ NS_WARNING("Disallowing adding non string/file types to DataTransfer");
+ aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
+ return nullptr;
+ }
+
+ if (aType.EqualsASCII(kFileMime) ||
+ aType.EqualsASCII(kFilePromiseMime)) {
+ NS_WARNING("Disallowing adding x-moz-file or x-moz-file-promize types to DataTransfer");
+ aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
+ return nullptr;
+ }
+ }
+
if (aIndex < mIndexedItems.Length()) {
nsTArray<RefPtr<DataTransferItem>>& items = mIndexedItems[aIndex];
uint32_t count = items.Length();
for (uint32_t i = 0; i < count; i++) {
RefPtr<DataTransferItem> item = items[i];
nsString type;
item->GetType(type);
if (type.Equals(aType)) {
@@ -486,30 +535,39 @@ DataTransferItemList::RegenerateFiles()
// list. That way we can avoid the unnecessary work if the user never touches
// the files list.
if (mFiles) {
// We clear the list rather than performing smaller updates, because it
// simplifies the logic greatly on this code path, which should be very
// infrequently used.
mFiles->Clear();
- uint32_t count = Length();
- for (uint32_t i = 0; i < count; i++) {
- ErrorResult rv;
- bool found;
- RefPtr<DataTransferItem> item = IndexedGetter(i, found, rv);
- if (NS_WARN_IF(!found || rv.Failed())) {
+ DataTransferItemList::GenerateFiles(mFiles, mFilesPrincipal);
+ }
+}
+
+void
+DataTransferItemList::GenerateFiles(FileList* aFiles,
+ nsIPrincipal* aFilesPrincipal)
+{
+ MOZ_ASSERT(aFiles);
+ MOZ_ASSERT(aFilesPrincipal);
+ uint32_t count = Length();
+ for (uint32_t i = 0; i < count; i++) {
+ ErrorResult rv;
+ bool found;
+ RefPtr<DataTransferItem> item = IndexedGetter(i, found, rv);
+ if (NS_WARN_IF(!found || rv.Failed())) {
+ continue;
+ }
+
+ if (item->Kind() == DataTransferItem::KIND_FILE) {
+ RefPtr<File> file = item->GetAsFileWithPrincipal(aFilesPrincipal, rv);
+ if (NS_WARN_IF(rv.Failed() || !file)) {
continue;
}
-
- if (item->Kind() == DataTransferItem::KIND_FILE) {
- RefPtr<File> file = item->GetAsFile(rv);
- if (NS_WARN_IF(rv.Failed() || !file)) {
- continue;
- }
- mFiles->Append(file);
- }
+ aFiles->Append(file);
}
}
}
} // namespace mozilla
} // namespace dom
--- a/dom/events/DataTransferItemList.h
+++ b/dom/events/DataTransferItemList.h
@@ -66,17 +66,17 @@ public:
void Clear(ErrorResult& aRv);
already_AddRefed<DataTransferItem>
SetDataWithPrincipal(const nsAString& aType, nsIVariant* aData,
uint32_t aIndex, nsIPrincipal* aPrincipal,
bool aInsertOnly, bool aHidden, ErrorResult& aRv);
- FileList* Files();
+ already_AddRefed<FileList> Files(nsIPrincipal* aPrincipal);
// Moz-style helper methods for interacting with the stored data
void MozRemoveByTypeAt(const nsAString& aType, uint32_t aIndex,
ErrorResult& aRv);
DataTransferItem* MozItemByTypeAt(const nsAString& aType, uint32_t aIndex);
const nsTArray<RefPtr<DataTransferItem>>* MozItemsAt(uint32_t aIndex);
uint32_t MozItemCount() const;
@@ -89,22 +89,25 @@ public:
private:
void ClearDataHelper(DataTransferItem* aItem, uint32_t aIndexHint,
uint32_t aMozOffsetHint, ErrorResult& aRv);
DataTransferItem* AppendNewItem(uint32_t aIndex, const nsAString& aType,
nsIVariant* aData, nsIPrincipal* aPrincipal,
bool aHidden);
void RegenerateFiles();
+ void GenerateFiles(FileList* aFiles, nsIPrincipal* aFilesPrincipal);
~DataTransferItemList() {}
RefPtr<DataTransfer> mDataTransfer;
bool mIsExternal;
RefPtr<FileList> mFiles;
+ // The principal for which mFiles is cached
+ nsCOMPtr<nsIPrincipal> mFilesPrincipal;
nsTArray<RefPtr<DataTransferItem>> mItems;
nsTArray<nsTArray<RefPtr<DataTransferItem>>> mIndexedItems;
};
} // namespace dom
} // namespace mozilla
#endif // mozilla_dom_DataTransferItemList_h
--- a/dom/events/test/test_DataTransferItemList.html
+++ b/dom/events/test/test_DataTransferItemList.html
@@ -74,16 +74,18 @@
is(files[0], file, "It should be the same file as the file we originally created");
is(file, e.dataTransfer.mozGetDataAt("text/html", 1),
"It should be stored in index 1 for mozGetDataAt");
var file2 = new File(['<a id="c"><b id="d">yo!</b></a>'], "myotherfile.html",
{type: "text/html"});
dtList.add(file2);
+ todo(files.length == 2, "This test has chrome privileges, so the FileList objects aren't updated live");
+ files = e.dataTransfer.files;
is(files.length, 2, "The files property should have been updated in place");
is(files[1], file2, "It should be the same file as the file we originally created");
is(file2, e.dataTransfer.mozGetDataAt("text/html", 2),
"It should be stored in index 2 for mozGetDataAt");
var oldLength = dtList.length;
var randomString = "foo!";
e.dataTransfer.mozSetDataAt("random/string", randomString, 3);
@@ -93,16 +95,19 @@
var file3 = new File(['<a id="e"><b id="f">heya!</b></a>'], "yetanotherfile.html",
{type: "text/html"});
e.dataTransfer.mozSetDataAt("random/string", file3, 3);
is(oldLength + 1, dtList.length,
"Replacing the entry with a file should add it to the list!");
is(dtList[oldLength].getAsFile(), file3, "It should be stored in the last index as a file");
is(dtList[oldLength].type, "random/string", "It should have the correct type");
is(dtList[oldLength].kind, "file", "It should have the correct kind");
+
+ todo(files.length == 3, "This test has chrome privileges, so the FileList objects aren't updated live");
+ files = e.dataTransfer.files;
is(files[files.length - 1], file3, "It should also be in the files list");
oldLength = dtList.length;
var nonstring = {};
e.dataTransfer.mozSetDataAt("jsobject", nonstring, 0);
is(oldLength + 1, dtList.length,
"Adding a non-string object using the mozAPIs to index 0 should add an item to the dataTransfer");
is(dtList[oldLength].type, "jsobject", "It should have the correct type");