Ver Fonte

Merge pull request #1765 from overleaf/bg-avoid-exception-in-token-access

handle exception in crypto.timingSafeEqual when checking token

GitOrigin-RevId: ba23aa4d4fe1e212c776965c0a58a90ecd846052
Brian Gough há 7 anos atrás
pai
commit
833caf4acd

+ 9 - 3
services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee

@@ -5,6 +5,7 @@ PrivilegeLevels = require '../Authorization/PrivilegeLevels'
 UserGetter = require '../User/UserGetter'
 UserGetter = require '../User/UserGetter'
 ObjectId = require("mongojs").ObjectId
 ObjectId = require("mongojs").ObjectId
 Settings = require('settings-sharelatex')
 Settings = require('settings-sharelatex')
+logger = require('logger-sharelatex')
 V1Api = require "../V1/V1Api"
 V1Api = require "../V1/V1Api"
 crypto = require 'crypto'
 crypto = require 'crypto'
 
 
@@ -41,10 +42,15 @@ module.exports = TokenAccessHandler =
 			return callback(err) if err?
 			return callback(err) if err?
 			if !project?
 			if !project?
 				return callback(null, null)
 				return callback(null, null)
-			if !crypto.timingSafeEqual(new Buffer(token), new Buffer(project.tokens.readAndWrite))
-				logger.err {token}, "read-and-write token match on numeric section, but not on full token"
+			try
+				if !crypto.timingSafeEqual(new Buffer(token), new Buffer(project.tokens.readAndWrite))
+					logger.err {token}, "read-and-write token match on numeric section, but not on full token"
+					return callback(null, null)
+				else
+					return callback(null, project)
+			catch err
+				logger.err {token, cryptoErr: err}, "error comparing tokens"
 				return callback(null, null)
 				return callback(null, null)
-			callback(null, project)
 
 
 	findProjectWithReadOnlyToken: (token, callback=(err, project, projectExists)->) ->
 	findProjectWithReadOnlyToken: (token, callback=(err, project, projectExists)->) ->
 		TokenAccessHandler._getProjectByReadOnlyToken token, (err, project) ->
 		TokenAccessHandler._getProjectByReadOnlyToken token, (err, project) ->

+ 14 - 0
services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee

@@ -150,6 +150,20 @@ describe "TokenAccessHandler", ->
 					expect(projectExists).to.equal true
 					expect(projectExists).to.equal true
 					done()
 					done()
 
 
+		describe 'when the tokens have different lengths', ->
+			beforeEach ->
+				@project.tokens = {
+					readOnly: 'atntntn'
+					readAndWrite: @token + "some-other-characters",
+					readAndWritePrefix: @tokenPrefix
+				}
+				@Project.findOne = sinon.stub().callsArgWith(2, null, @project)
+
+			it 'should not return a project', (done) ->
+				@TokenAccessHandler.findProjectWithReadAndWriteToken @token, (err, project) ->
+					expect(err).to.not.exist
+					expect(project).to.not.exist
+					done()	
 
 
 	describe 'findProjectWithHigherAccess', ->
 	describe 'findProjectWithHigherAccess', ->
 		describe 'when user does have higher access', ->
 		describe 'when user does have higher access', ->