pet-window.js Dynamic Code Evaluation: CWE-94 Hardening via Number

A critical defense-in-depth hardening landed in the pet-window module this week, closing a code injection path that could have let local attackers execute arbitrary JavaScript through manipulated configuration storage. While the maintainers note this wasn't demonstrated as exploitable in practice, the pattern—interpolating raw configuration values directly into dynamically generated code—violates fundamental security boundaries and creates an unbounded failure mode.

Affected Versions

The fix was proposed via automated security analysis and merged as a hardening measure at modules/pet-window.js:157 and surrounding lines.

The Vulnerability Explained

The pet-window module generates an HTML/JavaScript environment for rendering animated character assets. It builds this environment by concatenating base64-encoded image data and configuration parameters into a template string that becomes executable code.

The vulnerable pattern appeared in how numeric configuration values were handled. Consider the original code for display scaling:

var cfg_scale = config.display_scale || 1.0;

And for animation mixture and walk speed:

var renderMix = config.render_animation_mixture || 0.3;
var walkSpd = config.behavior_walk_speed || 30;

The deeper issue emerged with positional expressions. The code originally allowed JavaScript expressions to be embedded directly:

var posXExpr = (cfg_posX !== null && cfg_posX !== undefined) ? cfg_posX : "c.width/2";
var posYExpr = (cfg_posY !== null && cfg_posY !== undefined) ? cfg_posY : "c.height/5+c.height/20";

These values—cfg_posX, cfg_posY, cfg_scale, walkSpd, renderMix, and opacity—were ultimately interpolated into the generated JavaScript context. Because config was loaded from persistent storage without cryptographic verification, a local attacker with write access could modify these values to inject arbitrary JavaScript.

For example, setting config.display_scale to "1.0; require('child_process').exec('calc'); //" would have been interpolated directly into the generated code, with the || 1.0 fallback providing no protection against strings that begin with valid numbers.

The attack scenario requires local access because the configuration storage is typically protected by operating system permissions. However, in enterprise environments, shared workstations, or scenarios where application data directories are synchronized across devices (cloud storage, roaming profiles), this local boundary weakens considerably.

The Fix

The fix transforms every interpolated numeric parameter through explicit type coercion with validation:

Before:

var cfg_scale = config.display_scale || 1.0;
var renderMix = config.render_animation_mixture || 0.3;
var walkSpd = config.behavior_walk_speed || 30;
var opacity = config.opacity !== undefined ? config.opacity : 1.0;

After:

var cfg_scale = Number(config.display_scale) || 1.0;
var renderMix = Number(config.render_animation_mixture) || 0.3;
var walkSpd = Number(config.behavior_walk_speed) || 30;
var opacity = Number(config.opacity); if (isNaN(opacity)) opacity = 1.0;

The positional expressions received the most significant hardening. Where previously arbitrary JavaScript expressions could be injected:

Before:

var posXExpr = (cfg_posX !== null && cfg_posX !== undefined) ? cfg_posX : "c.width/2";
var posYExpr = (cfg_posY !== null && cfg_posY !== undefined) ? cfg_posY : "c.height/5+c.height/20";

After:

var posXExpr = "c.width/2";
if (cfg_posX !== null && cfg_posX !== undefined) { var _nx = Number(cfg_posX); if (!isNaN(_nx)) posXExpr = _nx; }
var posYExpr = "c.height/5+c.height/20";
if (cfg_posY !== null && cfg_posY !== undefined) { var _ny = Number(cfg_posY); if (!isNaN(_ny)) posYExpr = _ny; }

The fix achieves two critical objectives:

1. Type bounding: Number() coercion means only numeric values (or strings that parse as numbers) pass through. The string "1.0; maliciousCode();" becomes NaN, triggering the fallback.

2. NaN detection: For opacity, the fix explicitly checks isNaN() rather than relying on truthiness, since Number(undefined) is NaN (falsy) but Number("") is 0 (truthy)—a subtle distinction that could otherwise create unexpected behavior.

3. Expression elimination: The positional defaults are now hardcoded strings, with user input only acceptable as numeric overrides. The attack surface collapses from "arbitrary JavaScript expressions" to "numeric pixel coordinates."

Key Takeaways

• String interpolation into code is always dangerous: Even with || fallbacks, the JavaScript || operator only checks falsiness, not safety. A string beginning with a valid number passes through unchanged.

• Number() with isNaN() provides bounded failure: Unlike parseFloat() which extracts leading numbers from malicious strings, Number() returns NaN for any non-numeric input, enabling explicit rejection.

• Configuration storage needs the same scrutiny as network input: Local attackers with write access to application data are a real threat model in shared environments, container escapes, and supply chain scenarios.

• Defense-in-depth justifies hardening unproven vulnerabilities: The maintainers correctly note this wasn't demonstrated exploitable, yet the pattern's elimination prevents future security debt.

• Default expressions should not be user-overridable: The original design allowed users to override "c.width/2" with arbitrary expressions. The fix separates "trusted default logic" from "user-provided numeric overrides."

How Orbis AppSec Detected This

Source: The config object loaded from persistent storage, specifically properties display_scale, behavior_walk_speed, behavior_ai_activation, opacity, render_animation_mixture, posX, and posY

Sink: JavaScript code generation through template string concatenation in the pet window initialization, where configuration values were interpolated directly into executable code without transformation

Missing control: No type coercion or validation between configuration loading and code generation; the || fallback only provided default values, not sanitization

CWE: CWE-94 — Improper Control of Generation of Code ('Code Injection')

Fix: Replace direct interpolation with Number() coercion and explicit isNaN() validation, eliminating the expression-evaluation path for user-controlled positional parameters

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

This hardening of pet-window.js demonstrates how configuration-driven code generation creates subtle attack surfaces even in seemingly local, low-privilege contexts. The fix's pattern—Number() coercion with isNaN() validation—provides a reusable template for any JavaScript application that must safely incorporate external values into executable contexts. By making the failure mode explicit and bounded, the maintainers eliminated an entire class of potential injection vectors without changing the module's public API or user-visible behavior.