Skip to content

Commit

Permalink
Security Fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@sampaiodiego
sampaiodiego committed Feb 28, 2021
1 parent 1e36f41 commit 22b68a6
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 61 deletions.
5 changes: 4 additions & 1 deletion app/authorization/server/functions/hasRole.js
View file
@@ -1,7 +1,10 @@
import { Roles } from '../../../models/server/raw';

export const hasRoleAsync = async (userId, roleNames, scope) => {
roleNames = [].concat(roleNames);
if (!userId || userId === '') {
return false;
}

return Roles.isUserInRoles(userId, roleNames, scope);
};

Expand Down
8 changes: 4 additions & 4 deletions app/lib/server/functions/setUserAvatar.js
View file
Expand Up @@ -18,13 +18,13 @@ export const setUserAvatar = function(user, dataURI, contentType, service) {
try {
result = HTTP.get(dataURI, { npmRequestOptions: { encoding: 'binary', rejectUnauthorized: false } });
if (!result) {
console.log(`Not a valid response, from the avatar url: ${ dataURI }`);
throw new Meteor.Error('error-avatar-invalid-url', `Invalid avatar URL: ${ dataURI }`, { function: 'setUserAvatar', url: dataURI });
console.log(`Not a valid response, from the avatar url: ${ encodeURI(dataURI) }`);
throw new Meteor.Error('error-avatar-invalid-url', `Invalid avatar URL: ${ encodeURI(dataURI) }`, { function: 'setUserAvatar', url: dataURI });
}
} catch (error) {
if (!error.response || error.response.statusCode !== 404) {
console.log(`Error while handling the setting of the avatar from a url (${ dataURI }) for ${ user.username }:`, error);
throw new Meteor.Error('error-avatar-url-handling', `Error while handling avatar setting from a URL (${ dataURI }) for ${ user.username }`, { function: 'RocketChat.setUserAvatar', url: dataURI, username: user.username });
console.log(`Error while handling the setting of the avatar from a url (${ encodeURI(dataURI) }) for ${ user.username }:`, error);
throw new Meteor.Error('error-avatar-url-handling', `Error while handling avatar setting from a URL (${ encodeURI(dataURI) }) for ${ user.username }`, { function: 'RocketChat.setUserAvatar', url: dataURI, username: user.username });
}
}

Expand Down
18 changes: 6 additions & 12 deletions app/meteor-accounts-saml/server/lib/SAML.ts
View file
Expand Up @@ -372,7 +372,7 @@ export class SAML {
});
}

private static processValidateAction(req: IIncomingMessage, res: ServerResponse, service: IServiceProviderOptions, samlObject: ISAMLAction): void {
private static processValidateAction(req: IIncomingMessage, res: ServerResponse, service: IServiceProviderOptions, _samlObject: ISAMLAction): void {
const serviceProvider = new SAMLServiceProvider(service);
SAMLUtils.relayState = req.body.RelayState;
serviceProvider.validateResponse(req.body.SAMLResponse, (err, profile/* , loggedOut*/) => {
Expand All @@ -386,21 +386,15 @@ export class SAML {
throw new Error('No user data collected from IdP response.');
}

let credentialToken = (profile.inResponseToId && profile.inResponseToId.value) || profile.inResponseToId || profile.InResponseTo || samlObject.credentialToken;
// create a random token to store the login result
// to test an IdP initiated login on localhost, use the following URL (assuming SimpleSAMLPHP on localhost:8080):
// http://localhost:8080/simplesaml/saml2/idp/SSOService.php?spentityid=http://localhost:3000/_saml/metadata/test-sp
const credentialToken = Random.id();

const loginResult = {
profile,
};

if (!credentialToken) {
// If the login was initiated by the IDP, then we don't have a credentialToken as there was no AuthorizeRequest on our side
// so we create a random token now to use the same url to end the login
//
// to test an IdP initiated login on localhost, use the following URL (assuming SimpleSAMLPHP on localhost:8080):
// http://localhost:8080/simplesaml/saml2/idp/SSOService.php?spentityid=http://localhost:3000/_saml/metadata/test-sp
credentialToken = Random.id();
SAMLUtils.log('[SAML] Using random credentialToken: ', credentialToken);
}

this.storeCredential(credentialToken, loginResult);
const url = `${ Meteor.absoluteUrl('home') }?saml_idp_credentialToken=${ credentialToken }`;
res.writeHead(302, {
Expand Down
2 changes: 1 addition & 1 deletion app/meteor-accounts-saml/server/loginHandler.ts
View file
Expand Up @@ -11,7 +11,7 @@ const makeError = (message: string): Record<string, any> => ({
});

Accounts.registerLoginHandler('saml', function(loginRequest) {
if (!loginRequest.saml || !loginRequest.credentialToken) {
if (!loginRequest.saml || !loginRequest.credentialToken || typeof loginRequest.credentialToken !== 'string') {
return undefined;
}

Expand Down
6 changes: 4 additions & 2 deletions app/models/server/raw/Roles.js
View file
Expand Up @@ -8,13 +8,15 @@ export class RolesRaw extends BaseRaw {
}

async isUserInRoles(userId, roles, scope) {
roles = [].concat(roles);
if (!Array.isArray(roles)) {
roles = [roles];
}

for (let i = 0, total = roles.length; i < total; i++) {
const roleName = roles[i];

// eslint-disable-next-line no-await-in-loop
const role = await this.findOne({ _id: roleName });
const role = await this.findOne({ _id: roleName }, { scope: 1 });
const roleScope = (role && role.scope) || 'Users';
const model = this.models[roleScope];

Expand Down
84 changes: 43 additions & 41 deletions server/methods/addAllUserToRoom.js
View file
Expand Up @@ -11,52 +11,54 @@ Meteor.methods({
check(rid, String);
check(activeUsersOnly, Boolean);

if (hasRole(this.userId, 'admin') === true) {
const userCount = Users.find().count();
if (userCount > settings.get('API_User_Limit')) {
throw new Meteor.Error('error-user-limit-exceeded', 'User Limit Exceeded', {
method: 'addAllToRoom',
});
}
if (!hasRole(this.userId, 'admin')) {
throw new Meteor.Error(403, 'Access to Method Forbidden', {
method: 'addAllToRoom',
});
}

const room = Rooms.findOneById(rid);
if (room == null) {
throw new Meteor.Error('error-invalid-room', 'Invalid room', {
method: 'addAllToRoom',
});
}
const userFilter = {};
if (activeUsersOnly === true) {
userFilter.active = true;
}

const userFilter = {};
if (activeUsersOnly === true) {
userFilter.active = true;
}
const userCursor = Users.find(userFilter);
const usersCount = userCursor.count();
if (usersCount > settings.get('API_User_Limit')) {
throw new Meteor.Error('error-user-limit-exceeded', 'User Limit Exceeded', {
method: 'addAllToRoom',
});
}

const users = Users.find(userFilter).fetch();
const now = new Date();
users.forEach(function(user) {
const subscription = Subscriptions.findOneByRoomIdAndUserId(rid, user._id);
if (subscription != null) {
return;
}
callbacks.run('beforeJoinRoom', user, room);
Subscriptions.createWithRoomAndUser(room, user, {
ts: now,
open: true,
alert: true,
unread: 1,
userMentions: 1,
groupMentions: 0,
});
Messages.createUserJoinWithRoomIdAndUser(rid, user, {
ts: now,
});
Meteor.defer(function() {});
return callbacks.run('afterJoinRoom', user, room);
const room = Rooms.findOneById(rid);
if (!room) {
throw new Meteor.Error('error-invalid-room', 'Invalid room', {
method: 'addAllToRoom',
});
return true;
}
throw new Meteor.Error(403, 'Access to Method Forbidden', {
method: 'addAllToRoom',

const users = userCursor.fetch();
const now = new Date();
users.forEach(function(user) {
const subscription = Subscriptions.findOneByRoomIdAndUserId(rid, user._id);
if (subscription != null) {
return;
}
callbacks.run('beforeJoinRoom', user, room);
Subscriptions.createWithRoomAndUser(room, user, {
ts: now,
open: true,
alert: true,
unread: 1,
userMentions: 1,
groupMentions: 0,
});
Messages.createUserJoinWithRoomIdAndUser(rid, user, {
ts: now,
});
Meteor.defer(function() {});
return callbacks.run('afterJoinRoom', user, room);
});
return true;
},
});

0 comments on commit 22b68a6

Please sign in to comment.