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.

Sign up for GitHub

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

[feature] Allow using of the complex attribute names for encryptedNative and protectedNative #3032

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

[feature] Allow using of the complex attribute names for encryptedNative and protectedNative #3032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
121d282
Allow using of the complex attribute names for `encryptedNative` and …
GermanBluefox Mar 14, 2025
f3ebed9
Added tests
GermanBluefox Mar 14, 2025
5168d09
Fixing
GermanBluefox Mar 14, 2025
be4efa8
Fixing tests
GermanBluefox Mar 14, 2025
ddaa0cb
Merge branch 'master' into complex-encrypted
GermanBluefox Apr 2, 2025
9f446f0
Added changelog for 7.0.7
GermanBluefox Apr 2, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* (@foxriver76) fixed the edge-case problem on Windows (if adapter calls `readDir` on single file)
* (@foxriver76) fixed setting negative numbers via `state set` cli command
* (@GermanBluefox) corrected typing for `checkPasswordAsync` command and added caching of mulit-languages names
* (@GermanBluefox) Added support for the complex attribute names for the encrypted and protected native configuration properties: `encryptedNative` and `protectedNative`

## 7.0.6 (2024-12-08) - Lucy
* (@foxriver76) fixed UI upgrade if admin is running on privileged port (<1024)
Expand Down
20 changes: 11 additions & 9 deletions packages/adapter/src/lib/adapter/adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ import {
isMessageboxSupported,
listInstalledNodeModules,
requestModuleNameByUrl,
deleteObjectAttribute,
setObjectAttribute,
getObjectAttribute,
} from '@/lib/adapter/utils.js';

import type { Client as StatesInRedisClient } from '@iobroker/db-states-redis';
Expand Down Expand Up @@ -2583,7 +2586,7 @@ export class AdapterClass extends EventEmitter {

obj.native = mergedConfig;

return this.setForeignObjectAsync(configObjId, obj);
return this.setForeignObject(configObjId, obj);
}

/**
Expand Down Expand Up @@ -2627,7 +2630,7 @@ export class AdapterClass extends EventEmitter {
private async _getEncryptedConfig(options: InternalGetEncryptedConfigOptions): Promise<string | void> {
const { attribute, callback } = options;

const value = (this.config as InternalAdapterConfig)[attribute];
const value = getObjectAttribute(this.config, attribute);

if (typeof value === 'string') {
const secret = await this.getSystemSecret();
Expand Down Expand Up @@ -4431,7 +4434,7 @@ export class AdapterClass extends EventEmitter {
this.name !== id.split('.')[2]
) {
for (const attr of obj.protectedNative) {
delete obj.native[attr];
deleteObjectAttribute(obj.native, attr);
}
}
}
Expand Down Expand Up @@ -4600,7 +4603,7 @@ export class AdapterClass extends EventEmitter {
this.name !== obj._id.split('.')[2]
) {
for (const attr of obj.protectedNative) {
delete obj.native[attr];
deleteObjectAttribute(obj.native, attr);
}
}

Expand Down Expand Up @@ -11359,7 +11362,7 @@ export class AdapterClass extends EventEmitter {
this.name !== obj._id.split('.')[2]
) {
for (const attr of obj.protectedNative) {
delete obj.native[attr];
deleteObjectAttribute(obj.native, attr);
}
}

Expand Down Expand Up @@ -11457,7 +11460,7 @@ export class AdapterClass extends EventEmitter {
/**
* Initialize the adapter
*
* @param adapterConfig the AdapterOptions or the InstanceObject, is null/undefined if it is install process
* @param adapterConfig the AdapterOptions or the InstanceObject, is null/undefined if it is an installation process
*/
private async _initAdapter(adapterConfig?: AdapterOptions | ioBroker.InstanceObject | null): Promise<void> {
await this._initLogging();
Expand Down Expand Up @@ -11535,7 +11538,7 @@ export class AdapterClass extends EventEmitter {
instance = 0;
adapterConfig = adapterConfig || {
// @ts-expect-error protectedNative exists on instance objects
common: { mode: 'once', name: name, protectedNative: [] },
common: { mode: 'once', name, protectedNative: [] },
native: {},
};
}
Expand Down Expand Up @@ -11650,8 +11653,7 @@ export class AdapterClass extends EventEmitter {
if (typeof this.config[attr] === 'string') {
promises.push(
this.getEncryptedConfig(attr)
// @ts-expect-error
.then(decryptedValue => (this.config[attr] = decryptedValue))
.then(decryptedValue => setObjectAttribute(this.config, attr, decryptedValue))
.catch(e =>
this._logger.error(
`${this.namespaceLog} Can not decrypt attribute ${attr}: ${e.message}`,
Expand Down
79 changes: 75 additions & 4 deletions packages/adapter/src/lib/adapter/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@
const { secret, obj, keys } = options;

for (const attr of keys) {
const val = obj[attr];
const val = getObjectAttribute(obj, attr);
if (typeof val === 'string') {
obj[attr] = encrypt(secret, val);
setObjectAttribute(obj, attr, encrypt(secret, val));
}
}
}
Expand All @@ -80,9 +80,9 @@
const { secret, obj, keys } = options;

for (const attr of keys) {
const val = obj[attr];
const val = getObjectAttribute(obj, attr);
if (typeof val === 'string') {
obj[attr] = decrypt(secret, val);
setObjectAttribute(obj, attr, decrypt(secret, val));
}
}
}
Expand Down Expand Up @@ -149,3 +149,74 @@

return res.stdout.trim();
}

/**
* Get attribute of an object with complex names
*
* @param obj - object to get the attribute from
* @param attr - attribute name, can be complex like `attr1.attr2.attr3`
*/
export function getObjectAttribute(obj: Record<string, any>, attr: string): any {
if (!attr.includes('.')) {
return obj[attr];
}
const attrParts = attr.split('.');
for (let j = 0; j < attrParts.length; j++) {
if (j === attrParts.length - 1) {
return obj[attrParts[j]];
}
if (!obj[attrParts[j]] || typeof obj[attrParts[j]] !== 'object') {
break;
}
obj = obj[attrParts[j]];
}
}

/**
* Set attribute in an object with complex names
*
* @param obj - object to get the attribute from
* @param attr - attribute name, can be complex like `attr1.attr2.attr3`
* @param value - value to set
*/
export function setObjectAttribute(obj: Record<string, any>, attr: string, value: any): void {
if (!attr.includes('.')) {
obj[attr] = value;
return;
}
const attrParts = attr.split('.');
for (let j = 0; j < attrParts.length; j++) {
if (j === attrParts.length - 1) {
obj[attrParts[j]] = value;

Check warning

Code scanning / CodeQL

Prototype-polluting function Medium

The property chain
here
Loading
is recursively assigned to
obj
Loading
without guarding against prototype pollution.

Copilot Autofix

AI 26 days ago

To fix the problem, we need to ensure that the setObjectAttribute function does not allow setting properties like __proto__ or constructor that can lead to prototype pollution. We can achieve this by adding checks to skip these properties during the assignment process.

  • Modify the setObjectAttribute function to include checks that prevent setting __proto__ and constructor properties.
  • Ensure that these checks are applied both for direct assignments and for nested assignments within the loop.
Suggested changeset 1
packages/adapter/src/lib/adapter/utils.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/adapter/src/lib/adapter/utils.ts b/packages/adapter/src/lib/adapter/utils.ts
--- a/packages/adapter/src/lib/adapter/utils.ts
+++ b/packages/adapter/src/lib/adapter/utils.ts
@@ -188,2 +188,5 @@
     for (let j = 0; j < attrParts.length; j++) {
+        if (attrParts[j] === '__proto__' || attrParts[j] === 'constructor') {
+            return;
+        }
         if (j === attrParts.length - 1) {
EOF
@@ -188,2 +188,5 @@
for (let j = 0; j < attrParts.length; j++) {
if (attrParts[j] === '__proto__' || attrParts[j] === 'constructor') {
return;
}
if (j === attrParts.length - 1) {
Copilot is powered by AI and may make mistakes. Always verify output.
return;
}
if (!obj[attrParts[j]] || typeof obj[attrParts[j]] !== 'object') {
break;
}
obj = obj[attrParts[j]];
}
}

/**
* Delete attribute in an object with complex names
*
* @param obj - object to get the attribute from
* @param attr - attribute name, can be complex like `attr1.attr2.attr3`
*/
export function deleteObjectAttribute(obj: Record<string, any>, attr: string): void {
if (!attr.includes('.')) {
delete obj[attr];
return;
}
const attrParts = attr.split('.');
for (let j = 0; j < attrParts.length; j++) {
if (j === attrParts.length - 1) {
delete obj[attrParts[j]];
return;
}
if (!obj[attrParts[j]] || typeof obj[attrParts[j]] !== 'object') {
break;
}
obj = obj[attrParts[j]];
}
}
11 changes: 8 additions & 3 deletions packages/controller/test/lib/objects.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1171,15 +1171,20 @@
"paramBoolean": false,
"username": "tesla",
"password": "G\\\b\u000bY\fS",
"secondPassword": "$/aes-192-cbc:bcc46f0eec2addc9a54e70776c3f3f79:3861c124787cffb3e1a4e575f5acdb51a1e279ccde98a427946e1762075f051f"
"secondPassword": "$/aes-192-cbc:bcc46f0eec2addc9a54e70776c3f3f79:3861c124787cffb3e1a4e575f5acdb51a1e279ccde98a427946e1762075f051f",
"complex": {
"password": "G\\\b\u000bY\fS"
}
},
"protectedNative": [
"username",
"password"
"password",
"complex.password"
],
"encryptedNative": [
"password",
"secondPassword"
"secondPassword",
"complex.password"
],
"_id": "system.adapter.test.0",
"type": "adapter"
Expand Down
6 changes: 5 additions & 1 deletion packages/controller/test/lib/testAdapterHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont

const passphrase = 'SavePassword123';

await context.adapter.updateConfig({ secondPassword: passphrase });
await context.adapter.updateConfig({ secondPassword: passphrase, complex: { password: passphrase } });
const newConfig = await context.adapter.getForeignObjectAsync(`system.adapter.${context.adapter.namespace}`);

// non encrypted and non updated params stay the same
Expand All @@ -471,6 +471,10 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
// updated encrypted value is correctly decrypted
expect(newConfig?.native.secondPassword).to.exist;
expect(context.adapter.decrypt(newConfig?.native.secondPassword)).to.be.equal(passphrase);

// updated encrypted complex passwords, decrypt to the same value
expect(newConfig?.native.complex.password).to.exist;
expect(context.adapter.decrypt(newConfig?.native.complex.password)).to.be.equal(passphrase);
});

// setState object validation
Expand Down
28 changes: 22 additions & 6 deletions packages/controller/test/lib/testObjectsFunctions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,11 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
model: 'S P85D',
username: 'tesla',
password: 'winning',
complex: {
password: 'winning',
},
},
protectedNative: ['username', 'password'],
protectedNative: ['username', 'password', 'complex.password'],
objects: [],
instanceObjects: [],
},
Expand All @@ -377,6 +380,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
expect(obj!.native.model).equal('S P85D');
expect(obj!.native.username).to.be.undefined;
expect(obj!.native.password).to.be.undefined;
expect(obj!.native.complex.password).to.be.undefined;
expect(obj!._id).equal('system.adapter.tesla.0');
done();
});
Expand Down Expand Up @@ -406,8 +410,11 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
model: 'S P85D',
username: 'tesla',
password: 'winning',
complex: {
password: 'winning',
},
},
protectedNative: ['username', 'password'],
protectedNative: ['username', 'password', 'complex.password'],
objects: [],
instanceObjects: [],
},
Expand All @@ -422,6 +429,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
expect(obj!.native.model).equal('S P85D');
expect(obj!.native.password).equal('winning');
expect(obj!.native.username).equal('tesla');
expect(obj!.native.complex.password).equal('winning');
expect(obj!._id).equal(`system.adapter.${context.adapterShortName}.0`);
done();
});
Expand Down Expand Up @@ -994,11 +1002,11 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
},
},
() => {
// now create the enum with object as member
// now create the enum with an object as member
objects.setObject('enum.rooms.living_room', enumObj, () => {
// delete the object via adapter method
context.adapter.delForeignObject('tesla.0.model', () => {
// now get enum object
// now get an enum object
objects.getObject('enum.rooms.living_room', (err, obj) => {
// check that only the deleted object has been removed
expect((obj as ioBroker.EnumObject).common.members!.indexOf('tesla.0.model')).to.equal(-1);
Expand Down Expand Up @@ -1124,6 +1132,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
expect(obj!.native.model).equal('S P85D');
expect(obj!.native.username).to.be.undefined;
expect(obj!.native.password).to.be.undefined;
expect(obj!.native.complex.password).to.be.undefined;
expect(obj!._id).equal('system.adapter.tesla.0');
context.onAdapterObjectChanged = null;
done();
Expand All @@ -1147,8 +1156,11 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
model: 'S P85D',
username: 'tesla',
password: 'winning',
complex: {
password: 'winning',
},
},
protectedNative: ['username', 'password'],
protectedNative: ['username', 'password', 'complex.password'],
objects: [],
instanceObjects: [],
},
Expand All @@ -1171,6 +1183,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
expect(obj!.native.model).equal('S P85D');
expect(obj!.native.username).to.equal('tesla');
expect(obj!.native.password).to.equal('winning');
expect(obj!.native.complex.password).to.equal('winning');
expect(obj!._id).equal(`system.adapter.${context.adapterShortName}.0`);
context.onAdapterObjectChanged = null;
done();
Expand All @@ -1194,8 +1207,11 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
model: 'S P85D',
username: 'tesla',
password: 'winning',
complex: {
password: 'winning',
},
},
protectedNative: ['username', 'password'],
protectedNative: ['username', 'password', 'complex.password'],
objects: [],
instanceObjects: [],
},
Expand Down
Loading