Skip to content

Commit

Permalink
fix: vulnerability that allowed user to set acls for non-owned worlds
Browse files Browse the repository at this point in the history
  • Loading branch information
Mariano Goldman committed Jan 27, 2024
1 parent 8a9edd8 commit cc30c51
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/controllers/handlers/acl-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { AccessControlList, HandlerContextWithPath, PermissionType } from '../..
import { AuthChain, EthAddress } from '@dcl/schemas'
import { defaultPermissions } from '../../logic/permissions-checker'
import { InvalidRequestError } from '@dcl/platform-server-commons'
import { Authenticator } from '@dcl/crypto'

export async function getAclHandler(
ctx: HandlerContextWithPath<'worldsManager', '/acl/:world_name'>
Expand Down Expand Up @@ -35,6 +36,9 @@ export async function postAclHandler(
if (!AuthChain.validate(authChain)) {
throw new InvalidRequestError('Invalid payload received. Need to be a valid AuthChain.')
}
if (!Authenticator.isValidAuthChain(authChain)) {
throw new InvalidRequestError('Invalid payload received. Need to be a valid AuthChain.')
}

const permission = await namePermissionChecker.checkPermission(authChain[0].payload, worldName)
if (!permission) {
Expand Down
42 changes: 42 additions & 0 deletions test/integration/acl-handlers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,5 +336,47 @@ test('acl handlers', function ({ components, stubComponents }) {
message: `Timestamp is not recent. Please sign a new ACL change request.`
})
})

it.only('fails when the auth chain is invalid', async () => {
const delegatedIdentity = await getIdentity()

const timestamp = new Date().toISOString()
const payload = {
resource: worldName,
allowed: [delegatedIdentity.realAccount.address],
timestamp
}

const invalidAuthChain = [
{ type: 'SIGNER', payload: ownerIdentity.realAccount.address },
{ type: 'SIGNER', payload: JSON.stringify(payload) } // Auth Chain node is invalid
]

const r = await localFetch.fetch(`/acl/${worldName}`, {
body: JSON.stringify(invalidAuthChain),
method: 'POST'
})

expect(r.status).toEqual(400)
expect(await r.json()).toEqual({
error: 'Bad request',
message: `Invalid payload received. Need to be a valid AuthChain.`
})

const stored = await worldsManager.getMetadataForWorld(worldName)
expect(stored).toMatchObject({
permissions: {
...defaultPermissions(),
deployment: {
type: PermissionType.AllowList,
wallets: []
},
streaming: {
type: PermissionType.AllowList,
wallets: []
}
}
})
})
})
})

0 comments on commit cc30c51

Please sign in to comment.