Skip to content

Fix inconsistent profile check in API access - #3580

Open
Mips2648 wants to merge 10 commits into
developfrom
fix/control-jsonrpc-access
Open

Mips2648 wants to merge 10 commits into
developfrom
fix/control-jsonrpc-access

Conversation

@Mips2648

Copy link
Copy Markdown
Collaborator

Context

JSON-RPC and HTTP API methods had not enough or inconsistent profile check leading to potential vulnerability

Changes

JSON-RPC API (core/api/jeeApi.php)

  • Profile table: a single table maps each core method to the user profiles allowed to call it (admin, user, restrict).
  • When the check runs: once, after authentication and after the plugin API file has been included.
  • When it applies: only when the request uses a user API key ($_USER_GLOBAL is set).
  • Core API key and plugin keys: not affected.
  • Plugin methods (plugin != core): out of scope, each plugin keeps handling its own API.
  • Core method missing from the table: restricted to admin by default, so a method added later is protected even if someone forgets to add it to the table.
  • Duplicated checks: the per-method profile checks are removed. Per-object hasRight() checks on eqLogic, cmd and scenario are kept.
  • Methods aligned with the web UI (core/ajax/*.ajax.php):
    • config::byKey, config::save: now admin only (previously any profile).
    • log::get, log::getDelta, log::getLastLine, log::list: now admin only (previously admin, user). The api log records full requests, including API keys, so a user profile could read another user's or the core API key.
    • scenario::save, scenario::import: now admin only. scenario::hasRight() always returns true for the user profile, so any user could save a scenario with a PHP code block, i.e. execute code as www-data.
    • cmd::save: now admin only. Previously a user could change any existing command (pre/post actions, linked equipment, …).
    • update::all: now admin only (previously admin, user, restrict).

HTTP API (type=...)

  • type=message: now admin only, like message::add.
  • type=variable: reading requires admin or user (like datastore::byTypeLinkIdKey), writing requires admin (like datastore::save).
  • type=scenario without id, type=eqLogic, type=command: results are now filtered with hasRight(), like their JSON-RPC equivalents.

Shell injection hardening

  • jeedom::forceSyncHour(): ntp::optionalServer must be a valid hostname or IP address (filter_var). Otherwise the default NTP server is used. The value is then passed through escapeshellarg().
  • report::clean(): report::maxdays is checked with FILTER_VALIDATE_INT (min_range 0) and inserted with %d. An invalid or empty value now skips the cleanup, which matches the previous behaviour, where find failed on such values.

Breaking changes

  • Users with the user profile can no longer use these API methods: log::* (read), cmd::save, scenario::save, scenario::import, update::all.
  • Users with the restrict profile can no longer use update::all.
  • Over the HTTP API, non-admin users can no longer post messages (type=message) or write variables (type=variable). restrict users can no longer read variables.
  • Over the HTTP API, restrict users now only get the scenarios, equipments and commands they have rights on.
  • All profile denials now return error code -32701. Some methods used -32001 before (jeedom::halt/reboot/update/backup, eqLogic::save, cmd::save).
  • Plugin API keys are restricted by default and can no longer call core methods unless an admin explicitly unchecks "Restricted access" for that plugin.

Impact on json-rpc API

Method Current → Proposed Why
log::get, log::getDelta, log::getLastLine, log::list admin, user → admin The api log records each full request, including its apikey (log::add('api', 'info', secureXSS($request) ...)). A user can therefore read an admin API key or the core key and escalate to admin. The entire web interface for logs (log.ajax.php) is already restricted to admin.
scenario::save, scenario::import hasRight('w') → admin scenario::hasRight() always returns true for a user (scenario.class.php:1509-1511). Any user can therefore save a scenario containing a PHP code block, meaning arbitrary code execution as www-data. In the web interface, save already requires admin.
cmd::save hasRight (admin only at creation) → admin A user can modify any existing command: pre‑ and post‑execution actions, linking it to another device, etc. This allows triggering actions outside their permissions. In the web interface, save already requires admin.
update::all admin, user, restrict → admin This method exposes the inventory of plugins, their versions, and their sources, which is useful to identify a vulnerable plugin. In the web interface, update.ajax.php requires admin. It is also inconsistent that restrict can access it while user cannot access update::nbNeedUpdate.

Impact on HTTP API

Type Current → Proposed Why
variable (write) all → admin Aligned with datastore::save (admin).
variable (read) all → admin, user Aligned with datastore::byTypeLinkIdKey.
message all → admin Aligned with message::add.
scenario without id, object, eqLogic, command everything returned → filtering via hasRight Today a restrict user sees everything, whereas the equivalent JSON‑RPC methods already apply filtering.

Testing

  • php -l on the modified files.
  • Manual checks recommended:
    • core API key: full access to every method;
    • user keys with admin, user and restrict profiles on the methods listed above;
    • mobile app (getJson, event::changes, jeeObject::full, cmd::execCmd);
    • one plugin API with "Restricted access" checked, then unchecked.

Suggested changelog entry

  • Renforcement de la sécurité des accès API.

Types of changes

  • Bug fix (non-breaking change which fixes)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

@Mips2648 Mips2648 added this to the 5.0 milestone Oct 14, 2026
@Mips2648 Mips2648 added the changelog-breaking Use to generate release notes / changelog. To be apply on PR label Oct 14, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-breaking Use to generate release notes / changelog. To be apply on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant