Merge pull request #1115 from weseek/fix/cve-2019-13338

Fix/cve-2019-13338

CHANGES.md

@@ -2,7 +2,7 @@
 
 ## 3.5.3-RC
 
-* 
+* Fix: Create/Update page API returns data includes author's password hash
 
 ## 3.5.2
 

src/server/models/page.js

@@ -995,9 +995,8 @@ module.exports = function(crowi) {
     let savedPage = await page.save();
     const newRevision = Revision.prepareRevision(savedPage, body, null, user, { format });
     const revision = await pushRevision(savedPage, newRevision, user);
-    savedPage = await this.findByPath(revision.path)
-      .populate('revision')
-      .populate('creator');
+    savedPage = await this.findByPath(revision.path);
+    await savedPage.populateDataToShowRevision();
 
     if (socketClientId != null) {
       pageEvent.emit('create', savedPage, user, socketClientId);
@@ -1021,9 +1020,8 @@ module.exports = function(crowi) {
     let savedPage = await pageData.save();
     const newRevision = await Revision.prepareRevision(pageData, body, previousBody, user);
     const revision = await pushRevision(savedPage, newRevision, user);
-    savedPage = await this.findByPath(revision.path)
-      .populate('revision')
-      .populate('creator');
+    savedPage = await this.findByPath(revision.path);
+    await savedPage.populateDataToShowRevision();
 
     if (isSyncRevisionToHackmd) {
       savedPage = await this.syncRevisionToHackmd(savedPage);

src/server/models/user.js

@@ -65,6 +65,18 @@ module.exports = function(crowi) {
     createdAt: { type: Date, default: Date.now },
     lastLoginAt: { type: Date },
     admin: { type: Boolean, default: 0, index: true },
+  }, {
+    toObject: {
+      transform: (doc, ret, opt) => {
+        // omit password
+        delete ret.password;
+        // omit email
+        if (!doc.isEmailPublished) {
+          delete ret.email;
+        }
+        return ret;
+      },
+    },
   });
   userSchema.plugin(mongoosePaginate);
   userSchema.plugin(uniqueValidator);
@@ -374,24 +386,6 @@ module.exports = function(crowi) {
     return true;
   };
 
-  userSchema.statics.filterToPublicFields = function(user) {
-    debug('User is', typeof user, user);
-    if (typeof user !== 'object' || !user._id) {
-      return user;
-    }
-
-    const filteredUser = {};
-    const fields = USER_PUBLIC_FIELDS.split(' ');
-    for (let i = 0; i < fields.length; i++) {
-      const key = fields[i];
-      if (user[key]) {
-        filteredUser[key] = user[key];
-      }
-    }
-
-    return filteredUser;
-  };
-
   userSchema.statics.findUsers = function(options, callback) {
     const sort = options.sort || { status: 1, createdAt: 1 };
 
@@ -607,28 +601,18 @@ module.exports = function(crowi) {
     });
   };
 
-  userSchema.statics.resetPasswordByRandomString = function(id) {
-    const User = this;
+  userSchema.statics.resetPasswordByRandomString = async function(id) {
+    const user = await this.findById(id);
 
-    return new Promise(((resolve, reject) => {
-      User.findById(id, (err, userData) => {
-        if (!userData) {
-          return reject(new Error('User not found'));
-        }
+    if (!user) {
+      throw new Error('User not found');
+    }
 
-        // is updatable check
-        // if (userData.isUp
-        const newPassword = generateRandomTempPassword();
-        userData.setPassword(newPassword);
-        userData.save((err, userData) => {
-          if (err) {
-            return reject(err);
-          }
+    const newPassword = generateRandomTempPassword();
+    user.setPassword(newPassword);
+    await user.save();
 
-          resolve({ user: userData, newPassword });
-        });
-      });
-    }));
+    return newPassword;
   };
 
   userSchema.statics.createUsersByInvitation = function(emailList, toSendEmail, callback) {

src/server/routes/admin.js

@@ -577,19 +577,22 @@ module.exports = function(crowi, app) {
   };
 
   // app.post('/_api/admin/users.resetPassword' , admin.api.usersResetPassword);
-  actions.user.resetPassword = function(req, res) {
+  actions.user.resetPassword = async function(req, res) {
     const id = req.body.user_id;
     const User = crowi.model('User');
 
-    User.resetPasswordByRandomString(id)
-      .then((data) => {
-        data.user = User.filterToPublicFields(data.user);
-        return res.json(ApiResponse.success(data));
-      })
-      .catch((err) => {
-        debug('Error on reseting password', err);
-        return res.json(ApiResponse.error('Error'));
-      });
+    try {
+      const newPassword = await User.resetPasswordByRandomString(id);
+
+      const user = await User.findById(id);
+
+      const result = { user: user.toObject(), newPassword };
+      return res.json(ApiResponse.success(result));
+    }
+    catch (err) {
+      debug('Error on reseting password', err);
+      return res.json(ApiResponse.error(err));
+    }
   };
 
   actions.externalAccount = {};

src/server/routes/page.js

@@ -47,6 +47,14 @@ module.exports = function(crowi, app) {
     if (page.revisionHackmdSynced != null && page.revisionHackmdSynced._id != null) {
       returnObj.revisionHackmdSynced = page.revisionHackmdSynced._id;
     }
+
+    if (page.lastUpdateUser != null && page.lastUpdateUser instanceof User) {
+      returnObj.lastUpdateUser = page.lastUpdateUser.toObject();
+    }
+    if (page.creator != null && page.creator instanceof User) {
+      returnObj.creator = page.creator.toObject();
+    }
+
     return returnObj;
   }
 
@@ -585,8 +593,6 @@ module.exports = function(crowi, app) {
     }
 
     const result = { page: serializeToObj(createdPage), tags: savedTags };
-    result.page.lastUpdateUser = User.filterToPublicFields(createdPage.lastUpdateUser);
-    result.page.creator = User.filterToPublicFields(createdPage.creator);
     res.json(ApiResponse.success(result));
 
     // update scopes for descendants
@@ -674,7 +680,6 @@ module.exports = function(crowi, app) {
     }
 
     const result = { page: serializeToObj(page), tags: savedTags };
-    result.page.lastUpdateUser = User.filterToPublicFields(page.lastUpdateUser);
     res.json(ApiResponse.success(result));
 
     // update scopes for descendants