Aaron-Junker/sssm

Use `global` is very limiting and here is a risk of interfere

jakubboucek opened this issue · 1 comments

jakubboucek commented

here you are use global keyword to manage list of known states.

sssm/index.php

Line 30 in 716527c

global $$state;

This is very dangerous because this project is released as library. You state machine project can basically interfered with other values from host project.

And there is another problem with the global space:

$state1 = new state(["state2"]);

This is save variable to global space only when is this code called at top level of call stack (see backtrace). When you try to call this code at any function, it doesn't work.

This access can bring to code serious vulnerability with huge security breach.

When you wanna to keep simplicity of library, just create simple pool object which is collect all states.

class StatesPool {
    private static array $states = [];

    public static function add(string $name, state $state) {
        self::$states[$name] = $state;
    }

    public static function add(string $name) {
        if(isset(self::$states[$name]))
            return self::$states[$name];

        throw new Exception("No state with name $name in pool registered");
    }
}

And when you construct the state object, it is register self to pool:

    final public function __construct(string $name, array $allowedStatesAfter, bool $canLoop = true){
        $this->allowedStatesAfter = $allowedStatesAfter;
        $this->canLoop = $canLoop;
        StatesPool::add($name, $this);
    }

Now instead of:

$state1 = new state(["state2"]);

call:

new state('state1', ["state2"]);

Warning

This suggest is still very very dirty and is not recommended for any serious library! This issue is only discourages from using global a and suggest less-evil way.