Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent external domains from submitting more than one perm request at once #8148

Merged
merged 1 commit into from
Mar 6, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 46 additions & 8 deletions app/scripts/controllers/permissions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export class PermissionsController {
restrictedMethods: Object.keys(this._restrictedMethods),
store: this.store,
})
this.pendingApprovals = new Map()
this.pendingApprovalOrigins = new Set()
this._initializePermissions(restoredPermissions)
}

Expand Down Expand Up @@ -137,7 +139,7 @@ export class PermissionsController {
async approvePermissionsRequest (approved, accounts) {

const { id } = approved.metadata
const approval = this.pendingApprovals[id]
const approval = this.pendingApprovals.get(id)

if (!approval) {
log.warn(`Permissions request with id '${id}' not found`)
Expand All @@ -158,7 +160,7 @@ export class PermissionsController {
}))
}

delete this.pendingApprovals[id]
this._removePendingApproval(id)
}

/**
Expand All @@ -167,15 +169,15 @@ export class PermissionsController {
* @param {string} id - the id of the rejected request
*/
async rejectPermissionsRequest (id) {
const approval = this.pendingApprovals[id]
const approval = this.pendingApprovals.get(id)

if (!approval) {
log.warn(`Permissions request with id '${id}' not found`)
return
}

approval.reject(ethErrors.provider.userRejectedRequest())
delete this.pendingApprovals[id]
this._removePendingApproval(id)
}

/**
Expand Down Expand Up @@ -385,6 +387,38 @@ export class PermissionsController {
})
}

/**
* Adds a pending approval.
* @param {string} id - The id of the pending approval.
* @param {string} origin - The origin of the pending approval.
* @param {Function} resolve - The function resolving the pending approval Promise.
* @param {Function} reject - The function rejecting the pending approval Promise.
*/
_addPendingApproval (id, origin, resolve, reject) {

if (
this.pendingApprovalOrigins.has(origin) ||
this.pendingApprovals.has(id)
) {
throw new Error(
`Pending approval with id ${id} or origin ${origin} already exists.`
)
}

this.pendingApprovals.set(id, { origin, resolve, reject })
this.pendingApprovalOrigins.add(origin)
}

/**
* Removes the pending approval with the given id.
* @param {string} id - The id of the pending approval to remove.
*/
_removePendingApproval (id) {
const { origin } = this.pendingApprovals.get(id)
this.pendingApprovalOrigins.delete(origin)
this.pendingApprovals.delete(id)
}

/**
* A convenience method for retrieving a login object
* or creating a new one if needed.
Expand All @@ -396,8 +430,6 @@ export class PermissionsController {
// these permission requests are almost certainly stale
const initState = { ...restoredState, permissionsRequests: [] }

this.pendingApprovals = {}

this.permissions = new RpcCap({

// Supports passthrough methods:
Expand All @@ -418,12 +450,18 @@ export class PermissionsController {
* @param {string} req - The internal rpc-cap user request object.
*/
requestUserApproval: async (req) => {
const { metadata: { id } } = req
const { origin, metadata: { id } } = req

if (this.pendingApprovalOrigins.has(origin)) {
throw ethErrors.rpc.resourceUnavailable(
'Permission request already pending; please wait.'
)
}

this._platform.openExtensionInBrowser(`connect/${id}`)

return new Promise((resolve, reject) => {
this.pendingApprovals[id] = { resolve, reject }
this._addPendingApproval(id, origin, resolve, reject)
})
},
}, initState)
Expand Down