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

No input validation for getForeignObjectAsync() and possibly similar functions #947

Closed
Mic-M opened this issue Jun 19, 2020 · 6 comments
Closed

Comments

@Mic-M
Copy link
Contributor

Mic-M commented Jun 19, 2020

Hi :-)

Problem Statement:
If I call const lpStateObject = await this._adapter.getForeignObjectAsync(lpStatePath); so calling getForeignObjectAsync() without validating the input and providing an empty string:
image

try()/catch() jumps into catch().

This happens if the parameter for getForeignObjectAsync() is an empty string.

Suggested Solution
Please add a validation of the input parameter and throw an error if not met. In my Smart Control adapter, I will be now using the following function within a class (quickly put together, not extensively tested, yet).

    /**
     * Checks if an ioBroker state id (path) string is valid. 
     * NOTE: This does not verify if the state is existing.
     * 
     * @param {string}   str     String to validate
     * @return {boolean}         true/false of validation result
     */
    isStateIdValid(str) {

        // Source: https://github.com/ioBroker/ioBroker.js-controller/blob/master/lib/adapter.js - Version 3.1.6
        const FORBIDDEN_CHARS   = /[\][*,;'"`<>\\?]/g;

        // Is string?
        if (!str || typeof str !== 'string') return false;

        // Min length 5 chars? (minimum state length is 5, assuming this is a valid state: 'x.0.a'
        if(str.length < 5) return false;

        // No forbidden chars?
        if (FORBIDDEN_CHARS.test(str)) return false;

        // Do we have an instance number with a leading and trailing dot "."?
        if (!/\.\d+\./.test(str)) return false;

        // No space characters?
        const numberOfSpaces = (str.match(/\s/g) || []).length;
        if (numberOfSpaces > 0) return false;

        // Final
        return true;

    }

```
Thanks!
@foxriver76
Copy link
Collaborator

foxriver76 commented Jun 19, 2020

Hm there is a validation of the Id in getForeignObject since js-c 3..

validateId(id, true, options);

if (!id && id !== 0) {

@Mic-M
Copy link
Contributor Author

Mic-M commented Jun 19, 2020

Thank you @foxriver76 for looking into this!
See my code which I have just tested in a main.js of an adapter right at the beginning of "this.on('ready'...)":

class SmartControl extends utils.Adapter {

    /**
     * Constructor
     * 
     * @param {Partial<utils.AdapterOptions>} [options={}]  Adapter Options
     * 
     */
    constructor(options) {
        super( {...options, name: 'smartcontrol'} ); // to access the object's parent
        this.on('ready',        this._asyncOnReady.bind(this));
        this.on('stateChange',  this._asyncOnStateChange.bind(this));
        this.on('unload',       this._onUnload.bind(this));
    }

    /**
     * Called once ioBroker databases are connected and adapter received configuration.
     */
    async _asyncOnReady() {

        await test(this, '');
        this.log.info('----------------------------------');
        await test(this, 0);
        this.log.info('----------------------------------');
        await test(this, /hallo ioBroker/);
        this.log.info('----------------------------------');
        await test(this, '0_userdata.0.example_state');
        this.log.info('----------------------------------');



        async function test(adapter, input) {
            adapter.log.info(`Input: [${JSON.stringify(input)}]`);
            try {
                const ret = await adapter.getForeignObjectAsync(input);
                adapter.log.info(`getForeignObjectAsync() returns ${ret}`);
                return;
            } catch (error) {
                adapter.log.error(`=== CATCH === ${error.message}, stack: ${error.stack}`);
            }
        }



And this is the log:

2020-06-19 23:40:50.344  - info: smartcontrol.0 (13420) starting. Version 0.0.1 in C:/iobroker/node_modules/iobroker.smartcontrol, node: v12.16.2
2020-06-19 23:40:50.354  - info: smartcontrol.0 (13420) Input: [""]
2020-06-19 23:40:50.372  - error: smartcontrol.0 (13420) === CATCH === undefined, stack: undefined
2020-06-19 23:40:50.374  - info: smartcontrol.0 (13420) ----------------------------------
2020-06-19 23:40:50.375  - info: smartcontrol.0 (13420) Input: [0]
2020-06-19 23:40:50.377  - error: smartcontrol.0 (13420) === CATCH === undefined, stack: undefined
2020-06-19 23:40:50.378  - info: smartcontrol.0 (13420) ----------------------------------
2020-06-19 23:40:50.379  - info: smartcontrol.0 (13420) Input: [{}]
2020-06-19 23:40:50.380  - error: smartcontrol.0 (13420) === CATCH === undefined, stack: undefined
2020-06-19 23:40:50.381  - info: smartcontrol.0 (13420) ----------------------------------
2020-06-19 23:40:50.382  - info: smartcontrol.0 (13420) Input: ["0_userdata.0.example_state"]
2020-06-19 23:40:50.392  - info: smartcontrol.0 (13420) getForeignObjectAsync() returns [object Object]
2020-06-19 23:40:50.410  - info: smartcontrol.0 (13420) ----------------------------------

I am using VS Code under Windows, if this has any relevance to this issue.

@foxriver76
Copy link
Collaborator

Ah so you mean its not a real error which is thrown.. it is probably only a string in 3.1 but will be a real error in 3.2

So catching and logging only error instead of error.message should work.

@Apollon77
Copy link
Collaborator

Ahhh... try to log error directly please ... In js-controller <3.2 (which will come in September 2020) the return could be a string and not in any case an Error object

@Mic-M
Copy link
Contributor Author

Mic-M commented Jun 19, 2020

Thanks @Apollon77 and @foxriver76
So when using

adapter.log.error(`=== CATCH === ${JSON.stringify(error)}, stack: ${error.stack}`);

the log is as follows:

2020-06-19 23:58:43.561  - info: smartcontrol.0 (19100) starting. Version 0.0.1 in C:/iobroker/node_modules/iobroker.smartcontrol, node: v12.16.2
2020-06-19 23:58:43.572  - info: smartcontrol.0 (19100) Input: [""]
2020-06-19 23:58:43.596  - error: smartcontrol.0 (19100) === CATCH === "invalid id \"\"", stack: undefined
2020-06-19 23:58:43.629  - info: smartcontrol.0 (19100) ----------------------------------
2020-06-19 23:58:43.634  - info: smartcontrol.0 (19100) Input: [0]
2020-06-19 23:58:43.661  - error: smartcontrol.0 (19100) === CATCH === "invalid id 0", stack: undefined
2020-06-19 23:58:43.663  - info: smartcontrol.0 (19100) ----------------------------------
2020-06-19 23:58:43.665  - info: smartcontrol.0 (19100) Input: [{}]
2020-06-19 23:58:43.695  - error: smartcontrol.0 (19100) === CATCH === "invalid id {}", stack: undefined
2020-06-19 23:58:43.697  - info: smartcontrol.0 (19100) ----------------------------------
2020-06-19 23:58:43.725  - info: smartcontrol.0 (19100) Input: ["0_userdata.0.example_state"]
2020-06-19 23:58:43.749  - info: smartcontrol.0 (19100) getForeignObjectAsync() returns [object Object]
2020-06-19 23:58:43.775  - info: smartcontrol.0 (19100) ----------------------------------

@Mic-M
Copy link
Contributor Author

Mic-M commented Jun 19, 2020

thanks, I am having further questions and appreciate your response, see: https://forum.iobroker.net/topic/34569/adapter-error-handling-frage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants