Perform security fixes

Reviewed By: jeanlauliac Differential Revision: D14241523 fbshipit-source-id: 87c2f75861ade08e0a37cc6bc01b22ba08b008ef
facebook · Mar 4, 2019 · dcb41e3 · dcb41e3
1 parent 08f41b7
commit dcb41e3
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 14 deletions.
diff --git a/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap b/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap
@@ -33,7 +33,10 @@ Object {
       "mp3",
       "wav",
       "html",
+      "json",
       "pdf",
+      "yaml",
+      "yml",
       "otf",
       "ttf",
       "zip",
@@ -161,7 +164,10 @@ Object {
       "mp3",
       "wav",
       "html",
+      "json",
       "pdf",
+      "yaml",
+      "yml",
       "otf",
       "ttf",
       "zip",

diff --git a/packages/metro-config/src/defaults/defaults.js b/packages/metro-config/src/defaults/defaults.js
@@ -37,7 +37,10 @@ exports.assetExts = [
   'wav',
   // Document formats
   'html',
+  'json',
   'pdf',
+  'yaml',
+  'yml',
   // Font formats
   'otf',
   'ttf',

diff --git a/packages/metro/src/Assets.js b/packages/metro/src/Assets.js
@@ -265,6 +265,7 @@ async function getAsset(
   projectRoot: string,
   watchFolders: $ReadOnlyArray<string>,
   platform: ?string = null,
+  assetExts: $ReadOnlyArray<string>,
 ): Promise<Buffer> {
   const assetData = AssetPaths.parse(
     relativePath,
@@ -273,6 +274,12 @@ async function getAsset(
 
   const absolutePath = path.resolve(projectRoot, relativePath);
 
+  if (!assetExts.includes(assetData.type)) {
+    throw new Error(
+      `'${relativePath}' cannot be loaded as its extension is not registered in assetExts`,
+    );
+  }
+
   if (!pathBelongsToRoots(absolutePath, [projectRoot, ...watchFolders])) {
     throw new Error(
       `'${relativePath}' could not be found, because it cannot be found in the project root or any watch folder`,

diff --git a/packages/metro/src/Server.js b/packages/metro/src/Server.js
@@ -364,6 +364,7 @@ class Server {
         this._config.watchFolders,
         /* $FlowFixMe: query may be empty for invalid URLs */
         urlObj.query.platform,
+        this._config.resolver.assetExts,
       );
       // Tell clients to cache this for 1 year.
       // This is safe as the asset url contains a hash of the asset.

diff --git a/packages/metro/src/Server/__tests__/Server-test.js b/packages/metro/src/Server/__tests__/Server-test.js
@@ -630,6 +630,7 @@ describe('processRequest', () => {
           '/root',
           ['/root'],
           'ios',
+          expect.any(Array),
         );
         expect(value).toBe('i am image');
         done();
@@ -653,6 +654,7 @@ describe('processRequest', () => {
           '/root',
           ['/root'],
           'ios',
+          expect.any(Array),
         );
         expect(value).toBe(mockData.slice(0, 4));
         done();
@@ -674,6 +676,7 @@ describe('processRequest', () => {
           '/root',
           ['/root'],
           undefined,
+          expect.any(Array),
         );
         expect(value).toBe('i am image');
         done();

diff --git a/packages/metro/src/__tests__/Assets-test.js b/packages/metro/src/__tests__/Assets-test.js
@@ -33,12 +33,20 @@ describe('getAsset', () => {
     mkdirp.sync('/root/imgs');
   });
 
+  it('should fail if the extension is not registerd', async () => {
+    writeImages({'b.png': 'b image', 'b@2x.png': 'b2 image'});
+
+    expect(getAssetStr('imgs/b.png', '/root', [], ['jpg'])).rejects.toThrow(
+      Error,
+    );
+  });
+
   it('should work for the simple case', () => {
     writeImages({'b.png': 'b image', 'b@2x.png': 'b2 image'});
 
     return Promise.all([
-      getAssetStr('imgs/b.png', '/root', []),
-      getAssetStr('imgs/b@1x.png', '/root', []),
+      getAssetStr('imgs/b.png', '/root', [], null, ['png']),
+      getAssetStr('imgs/b@1x.png', '/root', [], null, ['png']),
     ]).then(resp => resp.forEach(data => expect(data).toBe('b image')));
   });
 
@@ -52,11 +60,11 @@ describe('getAsset', () => {
 
     expect(
       await Promise.all([
-        getAssetStr('imgs/b.png', '/root', [], 'ios'),
-        getAssetStr('imgs/b.png', '/root', [], 'android'),
-        getAssetStr('imgs/c.png', '/root', [], 'android'),
-        getAssetStr('imgs/c.png', '/root', [], 'ios'),
-        getAssetStr('imgs/c.png', '/root', []),
+        getAssetStr('imgs/b.png', '/root', [], 'ios', ['png']),
+        getAssetStr('imgs/b.png', '/root', [], 'android', ['png']),
+        getAssetStr('imgs/c.png', '/root', [], 'android', ['png']),
+        getAssetStr('imgs/c.png', '/root', [], 'ios', ['png']),
+        getAssetStr('imgs/c.png', '/root', [], null, ['png']),
       ]),
     ).toEqual([
       'b ios image',
@@ -74,8 +82,8 @@ describe('getAsset', () => {
     });
 
     return Promise.all([
-      getAssetStr('imgs/b.jpg', '/root', []),
-      getAssetStr('imgs/b.png', '/root', []),
+      getAssetStr('imgs/b.jpg', '/root', [], null, ['jpg']),
+      getAssetStr('imgs/b.png', '/root', [], null, ['png']),
     ]).then(data => expect(data).toEqual(['jpeg image', 'png image']));
   });
 
@@ -87,7 +95,9 @@ describe('getAsset', () => {
       'b@4.5x.png': 'b4.5 image',
     });
 
-    expect(await getAssetStr('imgs/b@3x.png', '/root', [])).toBe('b4 image');
+    expect(await getAssetStr('imgs/b@3x.png', '/root', [], null, ['png'])).toBe(
+      'b4 image',
+    );
   });
 
   it('should pick the bigger one with platform ext', async () => {
@@ -104,8 +114,8 @@ describe('getAsset', () => {
 
     expect(
       await Promise.all([
-        getAssetStr('imgs/b@3x.png', '/root', []),
-        getAssetStr('imgs/b@3x.png', '/root', [], 'ios'),
+        getAssetStr('imgs/b@3x.png', '/root', [], null, ['png']),
+        getAssetStr('imgs/b@3x.png', '/root', [], 'ios', ['png']),
       ]),
     ).toEqual(['b4 image', 'b4 ios image']);
   });
@@ -118,7 +128,13 @@ describe('getAsset', () => {
     });
 
     expect(
-      await getAssetStr('../anotherfolder/b.png', '/root', ['/anotherfolder']),
+      await getAssetStr(
+        '../anotherfolder/b.png',
+        '/root',
+        ['/anotherfolder'],
+        null,
+        ['png'],
+      ),
     ).toBe('b image');
   });
 
@@ -130,7 +146,7 @@ describe('getAsset', () => {
     });
 
     await expect(
-      getAssetStr('../anotherfolder/b.png', '/root', []),
+      getAssetStr('../anotherfolder/b.png', '/root', [], null, ['png']),
     ).rejects.toBeInstanceOf(Error);
   });
 });