PST-057: Sync now and Send everything have no per-account rate limit, while Test is throttled #786

New issue

Closed

opened 2026-10-10 10:56:29 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-10 10:56:29 +00:00

Owner

Found in the security audit of 2026-10-10 (SECURITY-AUDIT-2026-10-10.md), run against main at c30851b from 8 to 10 October 2026, as PST-057. Severity: Low.

The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it.

Severity: Low
Category: missing-rate-limit
Location: src/postulo/plugins/views.py (492-515)
Found by: ownership review of plugins-views

Who can do it, and what they need. Any signed-in account. Needs nothing beyond its own sync connections, which it may create without limit.

How it works.

Create many sync connections (each a distinct pk, each with the configured remote host; the guarded client restricts destinations to public hosts unless the operator allowed private ones).
POST connections:sync_now for each in a loop. Each request runs sync_connection inline: the plugin's sync() makes real CalDAV/CardDAV/HTTP requests to the configured server inside the web worker. The cache lease (cache.add(lease_key)) only stops two runs of the same connection overlapping; it does not bound how many connections, nor how often a connection is re-run once its lease is released at the end of each run.
connections:backfill likewise has no throttle; each press re-walks every document of the account and issues get_or_create per document per connection.

Impact. The instance can be driven to generate sustained outbound traffic against a third-party host chosen by the account holder (within the destination policy), and worker time is consumed inline. ConnectionTestView was given throttle.consume('connection-test', ...) under #232 because 'a test is a real request to somebody's server at a press of a button'; Sync now is the same shape and heavier. Narrow path and largely self-service, and DoS on the instance itself is out of scope, hence low.

Evidence.

views.py:412-416 `throttle.consume("connection-test", request.user, throttle.rate_for("POSTULO_CONNECTION_TEST_RATE"))` exists only in ConnectionTestView. views.py:504-508 `connection = get_object_or_404(self.get_queryset(), pk=pk); if refused := _switched_off(request, connection): return refused; report = sync_connection(connection)` with no throttle. syncing.py `sync_connection`: `if not cache.add(key, timezone.now().isoformat(), LEASE_SECONDS): return SyncReport(... already_running=True)` then `finally: cache.delete(key)` — the lease ends with the run.

Verification (verdict: confirmed; severity after verification: Low). Only the Sync now half is a meaningful gap, at views.py:492-515. Sync now lets an account holder bypass both the Test throttle and the 15-minute minimum interval the scheduler enforces. Backfill (views.py:454-487) makes no outbound call and is idempotent, so it should be dropped from the finding. Precondition: the administrator has installed a third-party sync plugin, because none ships in this repository. Remediation: spend a throttle.consume('sync-now', request.user, throttle.rate_for(...)) allowance in ConnectionSyncNowView.post, as ConnectionTestView does.

Suggested remediation. Apply throttle.consume with a setting such as POSTULO_CONNECTION_SYNC_RATE (and a looser one for backfill) keyed per account in both views, mirroring ConnectionTestView, and reuse its 'try again in N seconds' message; add the two addresses to tests/security/test_rate_limits.py.

Found in the security audit of 2026-10-10 (`SECURITY-AUDIT-2026-10-10.md`), run against `main` at `c30851b` from 8 to 10 October 2026, as **PST-057**. Severity: **Low**. The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it. --- - **Severity:** Low - **Category:** missing-rate-limit - **Location:** `src/postulo/plugins/views.py` (492-515) - **Found by:** ownership review of plugins-views **Who can do it, and what they need.** Any signed-in account. Needs nothing beyond its own sync connections, which it may create without limit. **How it works.** 1. Create many `sync` connections (each a distinct pk, each with the configured remote host; the guarded client restricts destinations to public hosts unless the operator allowed private ones). 2. POST `connections:sync_now` for each in a loop. Each request runs `sync_connection` inline: the plugin's `sync()` makes real CalDAV/CardDAV/HTTP requests to the configured server inside the web worker. The cache lease (`cache.add(lease_key)`) only stops two runs of the *same* connection overlapping; it does not bound how many connections, nor how often a connection is re-run once its lease is released at the end of each run. 3. `connections:backfill` likewise has no throttle; each press re-walks every document of the account and issues `get_or_create` per document per connection. **Impact.** The instance can be driven to generate sustained outbound traffic against a third-party host chosen by the account holder (within the destination policy), and worker time is consumed inline. `ConnectionTestView` was given `throttle.consume('connection-test', ...)` under #232 because 'a test is a real request to somebody's server at a press of a button'; Sync now is the same shape and heavier. Narrow path and largely self-service, and DoS on the instance itself is out of scope, hence low. **Evidence.** ```text views.py:412-416 `throttle.consume("connection-test", request.user, throttle.rate_for("POSTULO_CONNECTION_TEST_RATE"))` exists only in ConnectionTestView. views.py:504-508 `connection = get_object_or_404(self.get_queryset(), pk=pk); if refused := _switched_off(request, connection): return refused; report = sync_connection(connection)` with no throttle. syncing.py `sync_connection`: `if not cache.add(key, timezone.now().isoformat(), LEASE_SECONDS): return SyncReport(... already_running=True)` then `finally: cache.delete(key)` — the lease ends with the run. ``` **Verification** (verdict: confirmed; severity after verification: Low). Only the Sync now half is a meaningful gap, at views.py:492-515. Sync now lets an account holder bypass both the Test throttle and the 15-minute minimum interval the scheduler enforces. Backfill (views.py:454-487) makes no outbound call and is idempotent, so it should be dropped from the finding. Precondition: the administrator has installed a third-party sync plugin, because none ships in this repository. Remediation: spend a `throttle.consume('sync-now', request.user, throttle.rate_for(...))` allowance in `ConnectionSyncNowView.post`, as `ConnectionTestView` does. **Suggested remediation.** Apply `throttle.consume` with a setting such as `POSTULO_CONNECTION_SYNC_RATE` (and a looser one for backfill) keyed per account in both views, mirroring ConnectionTestView, and reuse its 'try again in N seconds' message; add the two addresses to tests/security/test_rate_limits.py.

tiagoagueda added the

bug

security

labels

2026-10-10 10:56:29 +00:00

tiagoagueda added this to the 0.6.0 milestone

2026-10-10 10:57:57 +00:00

tiagoagueda referenced this issue

2026-10-10 10:58:40 +00:00

Security audit of 2026-10-10: index of PST-001 to PST-098 (#730–#827) #828

tiagoagueda self-assigned this

2026-10-11 10:52:48 +00:00