Fix bucket ssrf (#666)

* FIX: 修复bucketname ssrf 问题

* FIX: 添加正则校验

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;