Fix some security issue

Signed-off-by: Alexandre Bodin <bodin.alex@gmail.com>
strapi · Jun 11, 2020 · ccb428e · ccb428e
1 parent 7120bb2
commit ccb428e
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 27 deletions.
diff --git a/packages/strapi-plugin-users-permissions/config/functions/bootstrap.js b/packages/strapi-plugin-users-permissions/config/functions/bootstrap.js
@@ -122,7 +122,6 @@ module.exports = async () => {
           message: `<p>We heard that you lost your password. Sorry about that!</p>
 
 <p>But don’t worry! You can use the following link to reset your password:</p>
-
 <p><%= URL %>?code=<%= TOKEN %></p>
 
 <p>Thanks.</p>`,

diff --git a/packages/strapi-plugin-users-permissions/config/policies/rateLimit.js b/packages/strapi-plugin-users-permissions/config/policies/rateLimit.js
@@ -22,7 +22,7 @@ module.exports = async (ctx, next) => {
       {
         interval: 1 * 60 * 1000,
         max: 5,
-        prefixKey: `${ctx.request.url}:${ctx.request.ip}`,
+        prefixKey: `${ctx.request.path}:${ctx.request.ip}`,
         message,
       },
       strapi.plugins['users-permissions'].config.ratelimit

diff --git a/packages/strapi-plugin-users-permissions/controllers/Auth.js b/packages/strapi-plugin-users-permissions/controllers/Auth.js
@@ -315,29 +315,21 @@ module.exports = {
       key: 'advanced',
     });
 
+    const userInfo = _.omit(user, ['password', 'resetPasswordToken', 'role', 'provider']);
+
     settings.message = await strapi.plugins['users-permissions'].services.userspermissions.template(
       settings.message,
       {
         URL: advanced.email_reset_password,
-        USER: _.omit(user.toJSON ? user.toJSON() : user, [
-          'password',
-          'resetPasswordToken',
-          'role',
-          'provider',
-        ]),
+        USER: userInfo,
         TOKEN: resetPasswordToken,
       }
     );
 
     settings.object = await strapi.plugins['users-permissions'].services.userspermissions.template(
       settings.object,
       {
-        USER: _.omit(user.toJSON ? user.toJSON() : user, [
-          'password',
-          'resetPasswordToken',
-          'role',
-          'provider',
-        ]),
+        USER: userInfo,
       }
     );
 
@@ -647,29 +639,21 @@ module.exports = {
       }
     });
 
+    const userInfo = _.omit(user, ['password', 'resetPasswordToken', 'role', 'provider']);
+
     settings.message = await strapi.plugins['users-permissions'].services.userspermissions.template(
       settings.message,
       {
         URL: `${strapi.config.server.url}/auth/email-confirmation`,
-        USER: _.omit(user.toJSON ? user.toJSON() : user, [
-          'password',
-          'resetPasswordToken',
-          'role',
-          'provider',
-        ]),
+        USER: userInfo,
         CODE: jwt,
       }
     );
 
     settings.object = await strapi.plugins['users-permissions'].services.userspermissions.template(
       settings.object,
       {
-        USER: _.omit(user.toJSON ? user.toJSON() : user, [
-          'password',
-          'resetPasswordToken',
-          'role',
-          'provider',
-        ]),
+        USER: userInfo,
       }
     );
 

diff --git a/packages/strapi-plugin-users-permissions/controllers/UsersPermissions.js b/packages/strapi-plugin-users-permissions/controllers/UsersPermissions.js
@@ -7,6 +7,7 @@
  */
 
 const _ = require('lodash');
+const { isValidEmailTemplate } = require('./validation/email-template');
 
 module.exports = {
   /**
@@ -196,14 +197,24 @@ module.exports = {
       return ctx.badRequest(null, [{ messages: [{ id: 'Cannot be empty' }] }]);
     }
 
+    const emailTemplates = ctx.request.body['email-templates'];
+
+    for (let key in emailTemplates) {
+      const template = emailTemplates[key].options.message;
+
+      if (!isValidEmailTemplate(template)) {
+        return ctx.badRequest(null, [{ messages: [{ id: 'Invalid template' }] }]);
+      }
+    }
+
     await strapi
       .store({
         environment: '',
         type: 'plugin',
         name: 'users-permissions',
         key: 'email',
       })
-      .set({ value: ctx.request.body['email-templates'] });
+      .set({ value: emailTemplates });
 
     ctx.send({ ok: true });
   },

diff --git a/...s/strapi-plugin-users-permissions/controllers/validation/__tests__/email-template.test.js b/...s/strapi-plugin-users-permissions/controllers/validation/__tests__/email-template.test.js
@@ -0,0 +1,28 @@
+'use strict';
+
+const { isValidEmailTemplate } = require('../email-template');
+
+describe('isValidEmailTemplate', () => {
+  test('Accepts one valid pattern', () => {
+    expect(isValidEmailTemplate('<%= CODE %>')).toBe(true);
+    expect(isValidEmailTemplate('<%=CODE%>')).toBe(true);
+  });
+
+  test('Refuses invalid patterns', () => {
+    expect(isValidEmailTemplate('<%- CODE %>')).toBe(false);
+    expect(isValidEmailTemplate('<% CODE %>')).toBe(false);
+    expect(isValidEmailTemplate('<%= <% CODE %> %>')).toBe(false);
+    expect(isValidEmailTemplate('<%- <% CODE %> %>')).toBe(false);
+    expect(isValidEmailTemplate('${ <% CODE %> }')).toBe(false);
+    expect(isValidEmailTemplate('<%CODE%>')).toBe(false);
+    expect(isValidEmailTemplate('${CODE}')).toBe(false);
+    expect(isValidEmailTemplate('${ CODE }')).toBe(false);
+  });
+
+  test('Fails on non authorized keys', () => {
+    expect(isValidEmailTemplate('<% random expression %>')).toBe(false);
+    expect(isValidEmailTemplate('<% random expression }%>')).toBe(false);
+    expect(isValidEmailTemplate('<% some.var.azdazd %>')).toBe(false);
+    expect(isValidEmailTemplate('<% function() %>')).toBe(false);
+  });
+});
diff --git a/packages/strapi-plugin-users-permissions/controllers/validation/email-template.js b/packages/strapi-plugin-users-permissions/controllers/validation/email-template.js
@@ -0,0 +1,30 @@
+'use strict';
+
+const _ = require('lodash');
+
+const invalidPatternsRegexes = [/<%[^=]([^<>%]*)%>/m, /\${([^{}]*)}/m];
+const authorizedKeys = ['URL', 'CODE', 'USER', 'USER.email', 'USER.username', 'TOKEN'];
+
+const isValidEmailTemplate = template => {
+  for (let reg of invalidPatternsRegexes) {
+    if (reg.test(template)) {
+      return false;
+    }
+  }
+
+  const matches = Array.from(template.matchAll(/<%=([^<>%=]*)%>/g));
+  for (let match of matches) {
+    const [, group] = match;
+    const trimGroup = _.trim(group);
+
+    if (!authorizedKeys.includes(trimGroup)) {
+      return false;
+    }
+  }
+
+  return true;
+};
+
+module.exports = {
+  isValidEmailTemplate,
+};