mirror of
https://github.com/cliffe/BreakEscape.git
synced 2026-02-21 11:18:08 +00:00
SECURITY: Enforce server-side validation for all door/container access
Critical security fix: Removed client-side lock state checking that allowed bypass of server validation. Clients can no longer manipulate lock states to gain unauthorized access. Previous vulnerability: - Client checked props.locked (client-side data) - If false, directly called notifyServerUnlock with method='unlocked' - Server trusted this without validating its own scenario data - Attacker could: set doorSprite.doorProperties.locked = false, then access New secure flow: - Client ALWAYS calls handleUnlock regardless of perceived lock state - handleUnlock calls server with method='unlocked' for unlocked items - Server ALWAYS validates against its own scenario_data - Server only grants access if item is actually unlocked in server state - Client state is never trusted for authorization decisions Changes: - doors.js: Removed client-side lock check, always call handleUnlock - unlock-system.js: Handle unlocked items by verifying with server - interactions.js: Removed client-side container lock check - interactions.js: Removed notifyServerForUnlockedContainer helper Security principle: Never trust the client. All authorization must be server-side based on server state, not client-reported state.
This commit is contained in:
@@ -570,16 +570,11 @@ async function handleDoorInteraction(doorSprite) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (props.locked) {
|
||||
console.log(`Door is locked. Type: ${props.lockType}, Requires: ${props.requires}`);
|
||||
// Use unified unlock system for consistent behavior with items
|
||||
handleUnlock(doorSprite, 'door');
|
||||
} else {
|
||||
console.log('Door is unlocked, notifying server to grant access');
|
||||
// Notify server to add room to unlockedRooms even for unlocked doors
|
||||
const serverResponse = await notifyServerUnlock(doorSprite, 'door', 'unlocked');
|
||||
openDoor(doorSprite);
|
||||
}
|
||||
// SECURITY: Always use server-side validation
|
||||
// Client cannot be trusted to determine lock state
|
||||
// The server will check its scenario data and validate accordingly
|
||||
console.log('Checking door access with server...');
|
||||
handleUnlock(doorSprite, 'door');
|
||||
}
|
||||
|
||||
// Function to unlock a door (called after successful unlock)
|
||||
|
||||
@@ -13,33 +13,6 @@ export function setGameInstance(gameInstance) {
|
||||
gameRef = gameInstance;
|
||||
}
|
||||
|
||||
// Helper function to notify server for unlocked container access
|
||||
async function notifyServerForUnlockedContainer(sprite) {
|
||||
const apiClient = window.ApiClient || window.APIClient;
|
||||
const gameId = window.breakEscapeConfig?.gameId;
|
||||
|
||||
if (!apiClient || !gameId) {
|
||||
console.warn('ApiClient or gameId not available, skipping server notification');
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const targetId = sprite.scenarioData?.id || sprite.scenarioData?.name || sprite.objectId;
|
||||
console.log(`Notifying server of unlocked container access: ${targetId}`);
|
||||
|
||||
const response = await apiClient.unlock('object', targetId, null, 'unlocked');
|
||||
|
||||
// If server returned contents, populate them
|
||||
if (response.hasContents && response.contents && sprite.scenarioData) {
|
||||
console.log('Server returned container contents:', response.contents);
|
||||
sprite.scenarioData.contents = response.contents;
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('Failed to notify server of unlocked container access:', error);
|
||||
// Continue anyway - don't block the user experience
|
||||
}
|
||||
}
|
||||
|
||||
// Helper function to calculate interaction distance with direction-based offset
|
||||
// Extends reach from the edge of the player sprite in the direction the player is facing
|
||||
function getInteractionDistance(playerSprite, targetX, targetY) {
|
||||
@@ -723,18 +696,11 @@ export function handleObjectInteraction(sprite) {
|
||||
if (data.type === 'suitcase' || data.type === 'briefcase' || data.type === 'bag1' || data.type === 'bin1' || data.contents) {
|
||||
console.log('CONTAINER ITEM INTERACTION', data);
|
||||
|
||||
// Check if container is locked
|
||||
if (data.locked === true) {
|
||||
console.log('CONTAINER LOCKED - UNLOCK SYSTEM WILL HANDLE', data);
|
||||
handleUnlock(sprite, 'item');
|
||||
return;
|
||||
}
|
||||
|
||||
// Container is unlocked (or has no lock) - notify server and launch the container minigame
|
||||
console.log('CONTAINER UNLOCKED/OPEN - NOTIFYING SERVER AND LAUNCHING MINIGAME', data);
|
||||
notifyServerForUnlockedContainer(sprite).then(() => {
|
||||
handleContainerInteraction(sprite);
|
||||
});
|
||||
// SECURITY: Always validate with server
|
||||
// Client cannot be trusted to determine lock state
|
||||
// The unlock system will handle both locked and unlocked containers via server validation
|
||||
console.log('Validating container access with server...');
|
||||
handleUnlock(sprite, 'item');
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -64,10 +64,10 @@ export function handleUnlock(lockable, type) {
|
||||
}
|
||||
|
||||
// Get lock requirements based on type
|
||||
const lockRequirements = type === 'door'
|
||||
const lockRequirements = type === 'door'
|
||||
? getLockRequirementsForDoor(lockable)
|
||||
: getLockRequirementsForItem(lockable);
|
||||
|
||||
|
||||
if (!lockRequirements) {
|
||||
console.log('NO LOCK REQUIREMENTS FOUND');
|
||||
return;
|
||||
@@ -77,8 +77,20 @@ export function handleUnlock(lockable, type) {
|
||||
// Use 'locked' field instead of 'requires' (which is filtered server-side for security)
|
||||
const isLocked = lockRequirements.locked !== false;
|
||||
|
||||
// SECURITY: If client thinks door is unlocked, verify with server
|
||||
if (!isLocked) {
|
||||
console.log('OBJECT NOT LOCKED');
|
||||
console.log('CLIENT SEES UNLOCKED - VERIFYING WITH SERVER');
|
||||
// Call server to verify and grant access
|
||||
notifyServerUnlock(lockable, type, 'unlocked').then(serverResponse => {
|
||||
if (serverResponse && serverResponse.success) {
|
||||
unlockTarget(lockable, type, lockable.layer, serverResponse);
|
||||
} else {
|
||||
window.gameAlert('Access denied', 'error', 'Error', 3000);
|
||||
}
|
||||
}).catch(error => {
|
||||
console.error('Server verification failed:', error);
|
||||
window.gameAlert('Failed to verify access', 'error', 'Error', 3000);
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user