diff --git a/xo-server/node_modules/@xen-orchestra/vmware-explorer/esxi.mjs b/@xen-orchestra/vmware-explorer/esxi.mjs index 498eb65..2a54705 100644 --- a/xo-server/node_modules/@xen-orchestra/vmware-explorer/esxi.mjs +++ b/xo-server/node_modules/@xen-orchestra/vmware-explorer/esxi.mjs @@ -17,6 +17,10 @@ import fs from 'node:fs/promises' const { info, warn } = createLogger('xo:vmware-explorer:esxi') +// reserved names that would let an attacker-controlled SOAP response repoint the +// prototype of a plain object instead of just setting a data property on it +const UNSAFE_KEYS = new Set(['__proto__', 'constructor', 'prototype']) + export const VDDK_LIB_DIR = '/usr/local/lib/vddk' export const VDDK_LIB_PATH = `${VDDK_LIB_DIR}/vmware-vix-disklib-distrib` let nbdPort = 11000 @@ -229,16 +233,26 @@ export default class Esxi extends EventEmitter { const returnObj = Array.isArray(result.returnval.objects) ? result.returnval.objects : [result.returnval.objects] returnObj.forEach(({ obj, propSet }) => { - objects[obj.$value] = {} + const objId = obj.$value + // obj.$value and propSet[].name come straight from the (attacker-controllable) SOAP + // response: reject __proto__/constructor/prototype so they can't be used to repoint + // the prototype of `objects` or of an individual result entry + if (UNSAFE_KEYS.has(objId)) { + return + } + objects[objId] = {} propSet = Array.isArray(propSet) ? propSet : [propSet] propSet.forEach(({ name, val }) => { + if (UNSAFE_KEYS.has(name)) { + return + } // don't care about the type for now delete val.attributes // a scalar value : simplify it if (val.$value) { - objects[obj.$value][name] = val.$value + objects[objId][name] = val.$value } else { - objects[obj.$value][name] = val + objects[objId][name] = val } }) }) diff --git a/xo-server/node_modules/@xen-orchestra/vmware-explorer/parsers/vmsd.mjs b/@xen-orchestra/vmware-explorer/parsers/vmsd.mjs index 1055b9d..0a46387 100644 --- a/xo-server/node_modules/@xen-orchestra/vmware-explorer/parsers/vmsd.mjs +++ b/xo-server/node_modules/@xen-orchestra/vmware-explorer/parsers/vmsd.mjs @@ -1,5 +1,9 @@ // the vmsd file contain the snapshot history of the VM , and their chaining +// reserved names that would let an untrusted vmsd line pollute Object.prototype +// (e.g. "snapshot0.__proto__.polluted = "x"") instead of just setting a data property +const UNSAFE_KEYS = new Set(['__proto__', 'constructor', 'prototype']) + function set(obj, keyPath, val) { const [key, ...other] = keyPath @@ -26,6 +30,9 @@ function set(obj, keyPath, val) { obj[label][index] = val } } else { + if (UNSAFE_KEYS.has(key)) { + return + } if (other.length) { // an object if (!obj[key]) { diff --git a/xo-server/node_modules/@xen-orchestra/vmware-explorer/parsers/vmx.mjs b/@xen-orchestra/vmware-explorer/parsers/vmx.mjs index 16e67ed..32afcd9 100644 --- a/xo-server/node_modules/@xen-orchestra/vmware-explorer/parsers/vmx.mjs +++ b/xo-server/node_modules/@xen-orchestra/vmware-explorer/parsers/vmx.mjs @@ -1,5 +1,9 @@ // the VMX file contains the VM metadata +// reserved names that would let an untrusted VMX line pollute Object.prototype +// (e.g. "__proto__.permission = "admin"") instead of just setting a data property +const UNSAFE_KEYS = new Set(['__proto__', 'constructor', 'prototype']) + function set(obj, keyPath, val) { let [key, ...other] = keyPath @@ -7,6 +11,9 @@ function set(obj, keyPath, val) { // it's an array let index ;[key, index] = key.split(':') + if (UNSAFE_KEYS.has(key)) { + return + } index = parseInt(index) if (!obj[key]) { // first time on this array @@ -29,6 +36,9 @@ function set(obj, keyPath, val) { } } else { // it's an object + if (UNSAFE_KEYS.has(key)) { + return + } if (!other.length) { // without descendant obj[key] = val diff --git a/xo-server/dist/api/user.mjs b/xo-server/dist/api/user.mjs index 48358b3..afc2ed1 100644 --- a/xo-server/dist/api/user.mjs +++ b/xo-server/dist/api/user.mjs @@ -64,8 +64,13 @@ export async function set({ if (permission && id === this.apiContext.user.id) { throw invalidParameters('a user cannot change its own permission'); } - } else if (email || password || permission) { - throw invalidParameters('this properties can only changed by an administrator'); + } else { + if (email || password || permission) { + throw invalidParameters('this properties can only changed by an administrator'); + } + if (id !== this.apiContext.user.id) { + throw invalidParameters('a user can only change its own properties'); + } } const user = await this.getUser(id); if (!isEmpty(user.authProviders) && (email !== undefined || password !== undefined)) { diff --git a/xo-server/dist/api/vm.mjs b/xo-server/dist/api/vm.mjs index 3785742..5320645 100644 --- a/xo-server/dist/api/vm.mjs +++ b/xo-server/dist/api/vm.mjs @@ -1527,6 +1527,11 @@ importFromEsxi.params = { optional: true } }; +importFromEsxi.resolve = { + network: ['network', 'network', 'administrate'], + sr: ['sr', 'SR', 'administrate'], + template: ['template', 'VM-template', 'administrate'] +}; export async function importMultipleFromEsxi({ concurrency, host, @@ -1646,6 +1651,11 @@ importMultipleFromEsxi.params = { optional: true } }; +importMultipleFromEsxi.resolve = { + network: ['network', 'network', 'administrate'], + sr: ['sr', 'SR', 'administrate'], + template: ['template', 'VM-template', 'administrate'] +}; export const attachDisk = defer(async function ($defer, { vm, vdi, diff --git a/xo-server/dist/models/user.mjs b/xo-server/dist/models/user.mjs index 5727f49..0f651b8 100644 --- a/xo-server/dist/models/user.mjs +++ b/xo-server/dist/models/user.mjs @@ -23,7 +23,7 @@ export class Users extends Collection { user.preferences = isEmpty(tmp = user.preferences) ? undefined : JSON.stringify(tmp); } _unserialize(user) { - if (user.permission === undefined) { + if (!Object.hasOwn(user, 'permission')) { user.permission = 'none'; } user.authProviders = parseProp('user', user, 'authProviders', undefined);