Rethinking Crime Formulas

[Inveteratus]

I've recently been asked to examine the crimes system in MCcodes; and thought I'd share some ideas I developed while trying to improve the system.

When using the staff console on a stock mccodes system to create a new crime, the formula for success is given by default as "((WILL*0.8)/2.5)+(LEVEL/4)" which is compared to rand(1, 100), with a value of less than or equal meaning success.

Complex? No really, confusing? Yes.

Okay, so what are the problems with this system?

I believe in the early days of mccodes, people were managing to inject expressions into the crime formula, and since this was passed through eval(), all sorts of fun and games ensued. Negating the problems of security, how can we improve the expression such that is potentially more sane, and fitting to our system?

The first thing is to use a proper expression evaluator; something that will sensible throw an exception if it cannot understand the expression under evaluation. I use Symfony's expression language for this -- https://symfony.com/doc/current/components/expression_language.html as it seems to cover all the use cases I need.The simplest use case would be:

use Symfony\Component\ExpressionLanguage\ExpressionLanguage;

$formula = '((WILL*0.8)/2.5)+(LEVEL/4)';
$context = [
    'WILL' => 100,   // This would in reality come from $ir['will']
    'LEVEL' => 1,    // This would in reality come from $ir['level']
];

$expressionLanguage = new ExpressionLanguage();
$successChance = $expressionLanguage->evaluate($formula, $context);

Which would, as a base chance, yield a success chance of 32.25%

The problem with this should be apparent, in that the stock system is very limited in the variables it passes to eval(), LEVEL, CRIMEXP, EXP, WILL and IQ. Okay, not difficult to expand, but it quickly gets out of hand. It would better to have the ability to pass the entire user in ($ir). The biggest problem is of course the use of the eval() method, which is something that you should probably strive to remove from all of your projects. As it executes PHP code directly, with no safety net it is obviously open to abuse at all levels so proper expression evaluators are a far better solution.

But there's a problem. Suppose we want a simple formula base on crime-experience. Maybe something like log(CRIMEXP + 1, 1.25) + 50, which; assuming a current crimexp of 0, would yield a base success chance of 50%. For this, we have to pass in a list of whitelisted functions to the expression evaluator:

use Symfony\Component\ExpressionLanguage\ExpressionFunctionProviderInterface;
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;

class CrimeFunctions implements ExpressionFunctionProviderInterface
{
    public function getFunctions(): array
    {
        return [
            ExpressionFunction::fromPhp('log'),
        ];
    }
}

$formula = 'log(CRIMEXP + 1, 1.25) + 50';
$context = [
    'CRIMEXP' => 0,	// This would in reality come from $ir['crimexp']
];

$expressionLanguage = new ExpressionLanguage(null, new CrimeFunctions());
$successChance = $expressionLanguage->evaluate($formula, $context);

Okay, so that's functions safely whitelisted, obviously, you would probably return a while set of useful functions from the CrimeFunctions->getFunctions() call.

As for passing in the entire object, that's easy enough:

 

use Symfony\Component\ExpressionLanguage\ExpressionFunctionProviderInterface;
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;

class CrimeFunctions implements ExpressionFunctionProviderInterface
{
    public function getFunctions(): array
    {
        return [
            ExpressionFunction::fromPhp('abs'),
            ExpressionFunction::fromPhp('ceil'),
            ExpressionFunction::fromPhp('exp'),
            ExpressionFunction::fromPhp('floor'),
            ExpressionFunction::fromPhp('log'),
            ExpressionFunction::fromPhp('max'),
            ExpressionFunction::fromPhp('min'),
            ExpressionFunction::fromPhp('pi'),
            ExpressionFunction::fromPhp('pow'),
            ExpressionFunction::fromPhp('sqrt'),
        ];
    }
}

$formula = 'log(user.crimexp + 1, 1.25) + 50';  // Note how we have change the name of the CRIMEXP field to user.crimexp
$context = ['user' => (object)$ir];             // Note we are passing in the entire user object this time

$expressionLanguage = new ExpressionLanguage(null, new CrimeFunctions());
$successChance = $expressionLanguage->evaluate($formula, $context);

Much better. We now have a safe expression evaluator, that allows us to use a number of whitelisted functions and has access to all the different fields within the user (and userstats) record. This in turn allows us for more complex expression that could initially be obtained; for example you may decide that you want some crimes to add in a degreee of intelligence:

log(user.crimexp + 1, 1.25) + log(user.IQ + 1, 2) + 50

or maybe add in a variety of attributes:

log(user.crimexp + 1, 1.25) + log(max(user.strength, user.agility) / 4 + 1) + 50

The possibilities become endless.

It's worth experimenting with a spreadsheet to determine the best formulas to use - too many sites use the same tired old ((WILL*0.8)/2.5)+(LEVEL/4) expression, maybe only altering the WILL multiplier which after a while becomes obvious. It's also a good idea to have something in place that a) ensures your players always have some chance of succeeding, and b) always have some chance of failing; i.e:

max(5, min(log(user.crimexp + 1, 1.25), 95))

This would ensure a 5% minimum success rate, but equally only a 95% maximum success rate.

Wrapping Up

Use an decent expression evaluator tool over the dangerous eval() function

Whitelist only the functions you need

Make expansion easy by passing in the user context as a whole rather than individual fields

Experiment with your formulas

Make both failure and success a possibility

N.B.

It has been pointed out that perhaps instead of passing in the entire $ir array as an object, you create a context that whitelists key fields to prevent passing sensitive fields like password/salt/email etc.

 

[AdamHull]

Very interesting read! May come in handy and will evaluate on my local copy 

[ags_cs4]

Nice thoughts i like how you found counter measures to some thing that have been broken for years, i started a similar advanced quest/crimes system on my GL RPG game last year (didn't finish it or i could show some of the code) so the idea was i make the formulas in hooks and pass the success rate but from what i read here it seems like the approach you took is the best to go with, got this idea too one can make a function with `str_replace()` to swap the Keys with the corresponding value and not sure bout this but if one can check if the final result still hold any characters then it throw an error or don't execute the crime

[Inveteratus]

The advantage of using a proper expression evaluator is that it actually understands expressions correctly - there's no need to check for odd characters, or indeed rogue functions as the expression is correctly interpreted with and abstract syntax tree, thus any parse errors in it will raise an exception, any unknown functions will raise an exception, and of course any unknown variables will raise an exception.

As a substitute for eval(), it's certainly one of the best solutions I've come across certainly in this use case. It also has the potential; to be used for setting player attributes whilst USEing an item. For example: ""user.hp = min(user.maxhp. user.hp + 3)"" ...though I've not fully tested this idea.

@ags_cs4 Simple str_replace'ments still yield an expression which may or not be valid; ie: any function call which takes a variable number of arguments might stress your str_replace/preg_replace statement, and of course 1e4 is a valid decimal number. This approach may work for small simple formulas, but when you start adding in complex function calls, then it gets unwieldy very fast not to mention difficult to validate.

Edited September 6, 2022 by Lacuna

[ags_cs4]

On 9/6/2022 at 10:45 AM, Lacuna said:

The advantage of using a proper expression evaluator is that it actually understands expressions correctly - there's no need to check for odd characters, or indeed rogue functions as the expression is correctly interpreted with and abstract syntax tree, thus any parse errors in it will raise an exception, any unknown functions will raise an exception, and of course any unknown variables will raise an exception.

As a substitute for eval(), it's certainly one of the best solutions I've come across certainly in this use case. It also has the potential; to be used for setting player attributes whilst USEing an item. For example: ""user.hp = min(user.maxhp. user.hp + 3)"" ...though I've not fully tested this idea.

@ags_cs4 Simple str_replace'ments still yield an expression which may or not be valid; ie: any function call which takes a variable number of arguments might stress your str_replace/preg_replace statement, and of course 1e4 is a valid decimal number. This approach may work for small simple formulas, but when you start adding in complex function calls, then it gets unwieldy very fast not to mention difficult to validate.

thats so true, what i found to be so facinating in GL engine is the hook system i recently did some work with personal Stats using hooks and it was amazing so i this idea might be good if you can make it but one thing is that you have to write each formule manulay and add it as a hook but that way it will be parsed the right way but what i think is best in this way is its more flexible you can make a calculation and pass that to the formula not just static data from the Database

new hook('crimes_formulas', function () {
	return [
		'name' => 'bank haking formula',
		// can pass more data not just userdata to the function
		"formula" => function ($user, $data) {
			// example of MCC formula ((IQ*0.8)/2.5)+(LEVEL/4)
			// converted to GL
			$value = (($user->info->US_iq * 0.8) / 2.5) + ($user->info->US_level / 4);
			return round($value);
		},
	];
});