Fix bucket ssrf (#666)

* FIX: 修复bucketname ssrf 问题

* FIX: 添加正则校验

Author: Ari1c

lib/bucket.js

@@ -1,6 +1,7 @@
 
 
 const assert = require('assert');
+const utils = require('./common/utils/checkBucketname');
 
 const proto = exports;
 
@@ -16,6 +17,16 @@ function toArray(obj) {
   return [obj];
 }
 
+/**
+ * check Bucket name
+ */
+
+proto._checkBucketName = function _checkBucketName(name) {
+  if (!utils._checkBucketName(name)) {
+    throw new Error('The bucket must be conform to the specifications');
+  }
+};
+
 /**
  * Bucket opertaions
  */
@@ -62,11 +73,11 @@ proto.listBuckets = async function listBuckets(query, options) {
 };
 
 proto.useBucket = function useBucket(name) {
-  this.options.bucket = name;
-  return this;
+  return this.setBucket(name);
 };
 
 proto.setBucket = function useBucket(name) {
+  this._checkBucketName(name);
   this.options.bucket = name;
   return this;
 };
@@ -76,6 +87,7 @@ proto.getBucket = function getBucket() {
 };
 
 proto.getBucketLocation = async function getBucketLocation(name, options) {
+  this._checkBucketName(name);
   name = name || this.getBucket();
   const params = this._bucketRequestParams('GET', name, 'location', options);
   params.successStatuses = [200];
@@ -88,6 +100,7 @@ proto.getBucketLocation = async function getBucketLocation(name, options) {
 };
 
 proto.getBucketInfo = async function getBucketInfo(name, options) {
+  this._checkBucketName(name);
   name = name || this.getBucket();
   const params = this._bucketRequestParams('GET', name, 'bucketInfo', options);
   params.successStatuses = [200];
@@ -100,6 +113,7 @@ proto.getBucketInfo = async function getBucketInfo(name, options) {
 };
 
 proto.putBucket = async function putBucket(name, options) {
+  this._checkBucketName(name);
   options = options || {};
   const params = this._bucketRequestParams('PUT', name, '', options);
 
@@ -131,6 +145,7 @@ proto.putBucket = async function putBucket(name, options) {
 };
 
 proto.deleteBucket = async function deleteBucket(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('DELETE', name, '', options);
   const result = await this.request(params);
   if (result.status === 200 || result.status === 204) {
@@ -144,6 +159,7 @@ proto.deleteBucket = async function deleteBucket(name, options) {
 // acl
 
 proto.putBucketACL = async function putBucketACL(name, acl, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('PUT', name, 'acl', options);
   params.headers = {
     'x-oss-acl': acl
@@ -157,6 +173,7 @@ proto.putBucketACL = async function putBucketACL(name, acl, options) {
 };
 
 proto.getBucketACL = async function getBucketACL(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('GET', name, 'acl', options);
   params.successStatuses = [200];
   params.xmlResponse = true;
@@ -174,6 +191,7 @@ proto.getBucketACL = async function getBucketACL(name, options) {
 // logging
 
 proto.putBucketLogging = async function putBucketLogging(name, prefix, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('PUT', name, 'logging', options);
   let xml = `${'<?xml version="1.0" encoding="UTF-8"?>\n<BucketLoggingStatus>\n' +
     '<LoggingEnabled>\n<TargetBucket>'}${name}</TargetBucket>\n`;
@@ -191,6 +209,7 @@ proto.putBucketLogging = async function putBucketLogging(name, prefix, options)
 };
 
 proto.getBucketLogging = async function getBucketLogging(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('GET', name, 'logging', options);
   params.successStatuses = [200];
   params.xmlResponse = true;
@@ -204,6 +223,7 @@ proto.getBucketLogging = async function getBucketLogging(name, options) {
 };
 
 proto.deleteBucketLogging = async function deleteBucketLogging(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('DELETE', name, 'logging', options);
   params.successStatuses = [204, 200];
   const result = await this.request(params);
@@ -215,6 +235,7 @@ proto.deleteBucketLogging = async function deleteBucketLogging(name, options) {
 // website
 
 proto.putBucketWebsite = async function putBucketWebsite(name, config, options) {
+  this._checkBucketName(name);
   // config: index, [error]
   const params = this._bucketRequestParams('PUT', name, 'website', options);
   config = config || {};
@@ -235,6 +256,7 @@ proto.putBucketWebsite = async function putBucketWebsite(name, config, options)
 };
 
 proto.getBucketWebsite = async function getBucketWebsite(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('GET', name, 'website', options);
   params.successStatuses = [200];
   params.xmlResponse = true;
@@ -247,6 +269,7 @@ proto.getBucketWebsite = async function getBucketWebsite(name, options) {
 };
 
 proto.deleteBucketWebsite = async function deleteBucketWebsite(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('DELETE', name, 'website', options);
   params.successStatuses = [204];
   const result = await this.request(params);
@@ -258,6 +281,7 @@ proto.deleteBucketWebsite = async function deleteBucketWebsite(name, options) {
 // lifecycle
 
 proto.putBucketLifecycle = async function putBucketLifecycle(name, rules, options) {
+  this._checkBucketName(name);
   // rules: [rule, ...]
   // rule: [id], prefix, status, expiration, [days or date]
   // status: 'Enabled' or 'Disabled'
@@ -287,6 +311,7 @@ proto.putBucketLifecycle = async function putBucketLifecycle(name, rules, option
 };
 
 proto.getBucketLifecycle = async function getBucketLifecycle(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('GET', name, 'lifecycle', options);
   params.successStatuses = [200];
   params.xmlResponse = true;
@@ -317,6 +342,7 @@ proto.getBucketLifecycle = async function getBucketLifecycle(name, options) {
 };
 
 proto.deleteBucketLifecycle = async function deleteBucketLifecycle(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('DELETE', name, 'lifecycle', options);
   params.successStatuses = [204];
   const result = await this.request(params);
@@ -326,6 +352,7 @@ proto.deleteBucketLifecycle = async function deleteBucketLifecycle(name, options
 };
 
 proto.putBucketCORS = async function putBucketCORS(name, rules, options) {
+  this._checkBucketName(name);
   rules = rules || [];
   assert(rules.length, 'rules is required');
   rules.forEach((rule) => {
@@ -371,6 +398,7 @@ proto.putBucketCORS = async function putBucketCORS(name, rules, options) {
 };
 
 proto.getBucketCORS = async function getBucketCORS(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('GET', name, 'cors', options);
   params.successStatuses = [200];
   params.xmlResponse = true;
@@ -394,6 +422,7 @@ proto.getBucketCORS = async function getBucketCORS(name, options) {
 };
 
 proto.deleteBucketCORS = async function deleteBucketCORS(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('DELETE', name, 'cors', options);
   params.successStatuses = [204];
   const result = await this.request(params);
@@ -405,6 +434,7 @@ proto.deleteBucketCORS = async function deleteBucketCORS(name, options) {
 // referer
 
 proto.putBucketReferer = async function putBucketReferer(name, allowEmpty, referers, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('PUT', name, 'referer', options);
   let xml = '<?xml version="1.0" encoding="UTF-8"?>\n<RefererConfiguration>\n';
   xml += `  <AllowEmptyReferer>${allowEmpty ? 'true' : 'false'}</AllowEmptyReferer>\n`;
@@ -428,6 +458,7 @@ proto.putBucketReferer = async function putBucketReferer(name, allowEmpty, refer
 };
 
 proto.getBucketReferer = async function getBucketReferer(name, options) {
+  this._checkBucketName(name);
   const params = this._bucketRequestParams('GET', name, 'referer', options);
   params.successStatuses = [200];
   params.xmlResponse = true;
@@ -446,6 +477,7 @@ proto.getBucketReferer = async function getBucketReferer(name, options) {
 };
 
 proto.deleteBucketReferer = async function deleteBucketReferer(name, options) {
+  this._checkBucketName(name);
   return await this.putBucketReferer(name, true, null, options);
 };
 

lib/common/utils/checkBucketname.js

@@ -0,0 +1,9 @@
+/**
+ * check Bucket name
+ */
+
+exports._checkBucketName = function (name) {
+  const bucketRegex = /^[a-z0-9][a-z0-9-]{1,61}[a-z0-9]$/;
+  const checkBucket = bucketRegex.test(name);
+  return checkBucket;
+};

test/node/bucket.test.js

@@ -48,6 +48,27 @@ describe('test/bucket.test.js', () => {
     await utils.cleanBucket(store, bucket);
   });
 
+  describe('setBucket()', () => {
+    it('should check bucket name', async () => {
+      try {
+        const name = 'ali-oss-test-bucket-/';
+        await store.setBucket(name);
+        throw new Error('should not run');
+      } catch (err) {
+        assert(err.message === 'The bucket must be conform to the specifications');
+      }
+    });
+  });
+
+  describe('getBucket()', () => {
+    it('should get bucket name', async () => {
+      const name = 'ali-oss-test-bucket';
+      await store.setBucket(name);
+      const res = store.getBucket();
+      assert.equal(res, name);
+    });
+  });
+
   describe('putBucket()', () => {
     let name;
     let archvieBucket;