Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TimerService device stores repeaters in an ever-growing array #4286

Closed
Chris-Hibbert opened this issue Jan 12, 2022 · 2 comments
Closed

TimerService device stores repeaters in an ever-growing array #4286

Chris-Hibbert opened this issue Jan 12, 2022 · 2 comments
Assignees
Labels
audit-zestival Vulnerability assessment of ERTP + Zoe bug Something isn't working resource-exhaustion Threats to availability from resource exhaustion attacks security SwingSet package: SwingSet
Milestone

Comments

@Chris-Hibbert
Copy link
Contributor

Describe the bug

The TimerService stores repeaters in an array that grows without bound. When repeaters are deleted, they are replaced in the array with null, so that indexing is preserved.

Vulnerability

Clients can create repeaters at low cost (particularly if they delete them quickly), but this causes the timer to grow the repeaters array to an arbitrary size.

This an issue for Zoe and other core services that rely on timers.

Expected behavior

Memory use shouldn't grow like that.

Additional context

Identified during the Zoe Purple team exercise

@Chris-Hibbert Chris-Hibbert added bug Something isn't working Zoe package: Zoe audit-zestival Vulnerability assessment of ERTP + Zoe security labels Jan 12, 2022
@zarutian
Copy link
Contributor

/me thinks about if and how #3612 would fare in this context. Perhaps it would only be the same issues that Notifiers and Subscriptions have?

I think this is mainly an implementation spefic issue.

@erights erights added the resource-exhaustion Threats to availability from resource exhaustion attacks label Jan 18, 2022
@Tartuffo Tartuffo added MN-1 and removed Zoe package: Zoe labels Jan 20, 2022
@Tartuffo Tartuffo added SwingSet package: SwingSet MN-1 and removed MN-1 labels Jan 20, 2022
@Tartuffo Tartuffo removed the MN-1 label Feb 7, 2022
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@warner warner changed the title TimerService stores repeaters in an ever-growing array TimerService device stores repeaters in an ever-growing array Jun 25, 2022
warner added a commit that referenced this issue Aug 11, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time. However Notifiers will always report a scheduled
time (some multiple of the interval). The opaque `updateCount` used in
Notifier updates is now a time value, not a counter, so user tests
should refrain from asserting sequentiality.

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling authority.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #5668
closes #5709
closes #4282
refs #4286
closes #4296
closes #5616
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 16, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #5668
closes #5709
closes #4282
refs #4286
closes #4296
closes #5616
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 16, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #5668
closes #5709
closes #4282
refs #4286
closes #4296
closes #5616
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 16, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #5668
closes #5709
closes #4282
refs #4286
closes #4296
closes #5616
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 16, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #4282
refs #4286
closes #4296
closes #5616
closes #5668
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 16, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #4282
refs #4286
closes #4296
closes #5616
closes #5668
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 16, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #4282
refs #4286
closes #4296
closes #5616
closes #5668
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 19, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #4282
refs #4286
closes #4296
closes #5616
closes #5668
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 19, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #4282
refs #4286
closes #4296
closes #5616
closes #5668
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 19, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #4282
refs #4286
closes #4296
closes #5616
closes #5668
closes #5709
refs #5798
warner added a commit that referenced this issue Aug 19, 2022
vat-timer is now fully virtualized, durablized, and upgradeable. RAM
usage should be O(N) in the number of:

* pending Promise wakeups (`wakeAt`, `delay`)
* active Notifier promises (`makeNotifier`)
* active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`)

Pending promises will be disconnected (rejected) during upgrade, as
usual.

All handlers and Promises will fire with the most recent timestamp
available, which (under load) may be somewhat later than the scheduled
wakeup time.

Until cancellation, Notifiers will always report a scheduled time
(i.e. `start` plus some multiple of the interval). The opaque
`updateCount` used in Notifier updates is a counter starting from 1n.
When a Notifier is cancelled, the final/"finish" value is the
timestamp of cancellation, which may or may not be a multiple of the
interval (and might be a duplicate of the last non-final value). Once
in the cancelled state, `getUpdateSince(anything)` yields `{ value:
cancellationTimestamp, updateCount: undefined }`, and the
corresponding `iterator.next()` resolves to `{ value:
cancellationTimestamp, done: true }`. Neither will ever reject their
Promises (except due to upgrade).

Asking for a wakeup in the past or present will fire immediately.

Most API calls will accept an arbitrary Far object as a CancelToken,
which can be used to cancel the wakeup/repeater. `makeRepeater` is the
exception.

This does not change the device-timer API or implementation, however
vat-timer now only uses a single device-side wakeup, and only exposes
a single handler object, to minimize the memory usage and object
retention by the device (since devices do not participate in GC).

This introduces a `Clock` which can return time values without also
providing scheduling authority, and a `TimerBrand` which can validate
time values without providing clock or scheduling
authority. Timestamps are not yet Branded, but the scaffolding is in
place.

`packages/SwingSet/tools/manual-timer.js` offers a manually-driven
timer service, which can help with unit tests.

closes #4282
refs #4286
closes #4296
closes #5616
closes #5668
closes #5709
refs #5798
@warner
Copy link
Member

warner commented Aug 19, 2022

With the closing of #5668, the timer device still suffers from this problem, but users no longer get access to it. The new timer vat stores Repeaters as virtual objects so they no longer consume RAM, only DB, and they should be deleted automatically when unreferenced and unscheduled.

I'll close this ticket now, because the core problem is resolved, and our next step will be to replace the timer device with a stripped down version that has no Repeaters at all, only a single wakeup that vat-timer uses to implement everything else.

@warner warner closed this as completed Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-zestival Vulnerability assessment of ERTP + Zoe bug Something isn't working resource-exhaustion Threats to availability from resource exhaustion attacks security SwingSet package: SwingSet
Projects
None yet
Development

No branches or pull requests

5 participants