From 46def44bb8640a258bcd33604c7bb0dd74e03079 Mon Sep 17 00:00:00 2001
From: Kyle Herock <kherock@gmail.com>
Date: Wed, 6 Feb 2019 20:56:24 -0500
Subject: [PATCH] additional validation and tests for CORS configuration files

---
 lib/cors.js                      | 12 ++++++
 test/resources/cors_invalid1.xml | 10 +++++
 test/resources/cors_invalid2.xml |  9 +++++
 test/resources/cors_invalid3.xml |  7 ++++
 test/test.js                     | 64 ++++++++++++++++++++++++++++++++
 5 files changed, 102 insertions(+)
 create mode 100644 test/resources/cors_invalid1.xml
 create mode 100644 test/resources/cors_invalid2.xml
 create mode 100644 test/resources/cors_invalid3.xml

diff --git a/lib/cors.js b/lib/cors.js
index 5962415b..1aed9016 100644
--- a/lib/cors.js
+++ b/lib/cors.js
@@ -5,6 +5,9 @@ const xml2js = require("xml2js");
 
 const templateBuilder = require("./xml-template-builder");
 
+// https://docs.aws.amazon.com/AmazonS3/latest/dev/cors.html#cors-allowed-methods
+const corsAllowedMethods = ["GET", "PUT", "POST", "DELETE", "HEAD"];
+
 function createWildcardRegExp(str, flags = "") {
   const parts = str.split("*");
   if (parts.length > 2)
@@ -32,6 +35,15 @@ module.exports = function cors(config) {
         );
       }
 
+      for (const method of rule.AllowedMethod) {
+        if (!corsAllowedMethods.includes(method)) {
+          throw new Error(
+            "Found unsupported HTTP method in CORS config. Unsupported method is " +
+              method
+          );
+        }
+      }
+
       // Keep track if the rule has the plain wildcard '*' origin since S3 responds with '*'
       // instead of echoing back the request origin in this case
       rule.hasWildcardOrigin = rule.AllowedOrigin.includes("*");
diff --git a/test/resources/cors_invalid1.xml b/test/resources/cors_invalid1.xml
new file mode 100644
index 00000000..4ccf7743
--- /dev/null
+++ b/test/resources/cors_invalid1.xml
@@ -0,0 +1,10 @@
+<CORSConfiguration>
+    <CORSRule>
+        <!-- Only one wildcard is allowed -->
+        <AllowedOrigin>https://*.*</AllowedOrigin>
+        <AllowedMethod>HEAD</AllowedMethod>
+        <AllowedMethod>GET</AllowedMethod>
+        <MaxAgeSeconds>3000</MaxAgeSeconds>
+        <AllowedHeader>*</AllowedHeader>
+    </CORSRule>
+</CORSConfiguration>
diff --git a/test/resources/cors_invalid2.xml b/test/resources/cors_invalid2.xml
new file mode 100644
index 00000000..6f7a9d0e
--- /dev/null
+++ b/test/resources/cors_invalid2.xml
@@ -0,0 +1,9 @@
+<CORSConfiguration>
+    <CORSRule>
+        <AllowedOrigin>*</AllowedOrigin>
+        <!-- Method must be one of GET, PUT, POST, DELETE, HEAD -->
+        <AllowedMethod>*</AllowedMethod>
+        <MaxAgeSeconds>3000</MaxAgeSeconds>
+        <AllowedHeader>*</AllowedHeader>
+    </CORSRule>
+</CORSConfiguration>
diff --git a/test/resources/cors_invalid3.xml b/test/resources/cors_invalid3.xml
new file mode 100644
index 00000000..0b8710c1
--- /dev/null
+++ b/test/resources/cors_invalid3.xml
@@ -0,0 +1,7 @@
+<CORSConfiguration>
+    <CORSRule>
+        <!-- At least one AllowedOrigin or AllowedMethod must be specified -->
+        <MaxAgeSeconds>3000</MaxAgeSeconds>
+        <AllowedHeader>*</AllowedHeader>
+    </CORSRule>
+</CORSConfiguration>
diff --git a/test/test.js b/test/test.js
index 73b88134..b4ace653 100644
--- a/test/test.js
+++ b/test/test.js
@@ -1117,6 +1117,70 @@ describe("S3rver CORS Policy Tests", function() {
     }
   });
 
+  it("should fail to initialize a configuration with multiple wildcard characters", function*() {
+    let error;
+    try {
+      let server;
+      yield thunkToPromise(done => {
+        server = new S3rver({
+          port: 4569,
+          hostname: "localhost",
+          silent: true,
+          cors: fs.readFileSync("./test/resources/cors_invalid1.xml")
+        }).run(done);
+      });
+      yield thunkToPromise(done => server.close(done));
+    } catch (err) {
+      error = err;
+    }
+    expect(error).to.exist;
+    expect(error.message).to.include(" can not have more than one wildcard.");
+  });
+
+  it("should fail to initialize a configuration with an illegal AllowedMethod", function*() {
+    let error;
+    try {
+      let server;
+      yield thunkToPromise(done => {
+        server = new S3rver({
+          port: 4569,
+          hostname: "localhost",
+          silent: true,
+          cors: fs.readFileSync("./test/resources/cors_invalid2.xml")
+        }).run(done);
+      });
+      yield thunkToPromise(done => server.close(done));
+    } catch (err) {
+      error = err;
+    }
+    expect(error).to.exist;
+    expect(error.message).to.include(
+      "Found unsupported HTTP method in CORS config."
+    );
+  });
+
+  it("should fail to initialize a configuration with missing required fields", function*() {
+    let error;
+    try {
+      let server;
+      yield thunkToPromise(done => {
+        server = new S3rver({
+          port: 4569,
+          hostname: "localhost",
+          silent: true,
+          cors: fs.readFileSync("./test/resources/cors_invalid3.xml")
+        }).run(done);
+      });
+      yield thunkToPromise(done => server.close(done));
+    } catch (err) {
+      error = err;
+    }
+    expect(error).to.exist;
+    expect(error.message).to.include(
+      "CORSRule must have at least one AllowedOrigin and AllowedMethod"
+    );
+  });
+
   it("should add the Access-Control-Allow-Origin header for default (wildcard) configurations", function*() {
     const origin = "http://a-test.example.com";
     const params = { Bucket: bucket, Key: "image" };