Bug 1501154 - Disallow the empty string as a module specifier in the shell and improve error handling in js::shell::FileAsString() r=bbouvier
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 24 Oct 2018 14:29:21 +0100
changeset 491100 6f6a67ed98c7b70834768003df50993873e1a1de
parent 491099 034e53d3e7f06e67b30811198f020ad2cfa69998
child 491101 547f834561c3d106a8251922518158a8ca0aacc3
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersbbouvier
bugs1501154
milestone65.0a1
Bug 1501154 - Disallow the empty string as a module specifier in the shell and improve error handling in js::shell::FileAsString() r=bbouvier
js/src/jit-test/tests/modules/bug-1501154.js
js/src/shell/ModuleLoader.js
js/src/shell/js.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/modules/bug-1501154.js
@@ -0,0 +1,25 @@
+// Test using an empty string as a module specifier fails.
+let result = null;
+let error = null;
+let promise = import("");
+promise.then((ns) => {
+    result = ns;
+}).catch((e) => {
+    error = e;
+});
+
+drainJobQueue();
+assertEq(result, null);
+assertEq(error instanceof Error, true);
+
+// Test reading a directory as a file fails.
+result = null;
+error = null;
+try {
+    result = os.file.readFile(".");
+} catch (e) {
+    error = e;
+}
+
+assertEq(result, null);
+assertEq(error instanceof Error, true);
--- a/js/src/shell/ModuleLoader.js
+++ b/js/src/shell/ModuleLoader.js
@@ -33,16 +33,20 @@ const ReflectLoader = new class {
         this.loadPath = getModuleLoadPath();
     }
 
     isJavascriptURL(name) {
         return ReflectApply(StringPrototypeStartsWith, name, [JAVASCRIPT_SCHEME]);
     }
 
     resolve(name, referencingInfo) {
+        if (name === "") {
+            throw new ErrorClass("Invalid module specifier");
+        }
+
         if (this.isJavascriptURL(name) || os.path.isAbsolute(name)) {
             return name;
         }
 
         let loadPath = this.loadPath;
 
         // Treat |name| as a relative path if it starts with either "./"
         // or "../".
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -2271,37 +2271,55 @@ js::shell::FileAsString(JSContext* cx, J
     file = fopen(pathname.get(), "rb");
     if (!file) {
         ReportCantOpenErrorUnknownEncoding(cx, pathname.get());
         return nullptr;
     }
 
     AutoCloseFile autoClose(file);
 
+    struct stat st;
+    if (fstat(fileno(file), &st) != 0) {
+        JS_ReportErrorUTF8(cx, "can't stat %s", pathname.get());
+        return nullptr;
+    }
+
+    if ((st.st_mode & S_IFMT) != S_IFREG) {
+        JS_ReportErrorUTF8(cx, "can't read non-regular file %s", pathname.get());
+        return nullptr;
+    }
+
     if (fseek(file, 0, SEEK_END) != 0) {
         pathname = JS_EncodeStringToUTF8(cx, pathnameStr);
         if (!pathname) {
             return nullptr;
         }
         JS_ReportErrorUTF8(cx, "can't seek end of %s", pathname.get());
         return nullptr;
     }
 
-    size_t len = ftell(file);
+    long endPos = ftell(file);
+    if (endPos < 0) {
+        JS_ReportErrorUTF8(cx, "can't read length of %s", pathname.get());
+        return nullptr;
+    }
+
+    size_t len = endPos;
     if (fseek(file, 0, SEEK_SET) != 0) {
         pathname = JS_EncodeStringToUTF8(cx, pathnameStr);
         if (!pathname) {
             return nullptr;
         }
         JS_ReportErrorUTF8(cx, "can't seek start of %s", pathname.get());
         return nullptr;
     }
 
     UniqueChars buf(js_pod_malloc<char>(len + 1));
     if (!buf) {
+        JS_ReportErrorUTF8(cx, "out of memory reading %s", pathname.get());
         return nullptr;
     }
 
     size_t cc = fread(buf.get(), 1, len, file);
     if (cc != len) {
         if (ptrdiff_t(cc) < 0) {
             ReportCantOpenErrorUnknownEncoding(cx, pathname.get());
         } else {