Skip to content

fix: stop distribute() from looping forever on a non-positive count#506

Open
spokodev wants to merge 1 commit into
scurker:mainfrom
spokodev:fix-distribute-infinite-loop
Open

fix: stop distribute() from looping forever on a non-positive count#506
spokodev wants to merge 1 commit into
scurker:mainfrom
spokodev:fix-distribute-infinite-loop

Conversation

@spokodev

Copy link
Copy Markdown

Problem

distribute(count) loops with for (; count !== 0; count--), which only reaches 0 for a positive integer count. A negative or fractional count decrements past 0 forever, pushing currency objects into the result array until the process runs out of memory:

const currency = require('currency.js');

currency(1).distribute(-1);   // never returns -> FATAL ERROR: heap out of memory
currency(1).distribute(2.5);  // never returns -> FATAL ERROR: heap out of memory

count is part of the public, number-typed API (distribute(count: number)), so a caller-supplied (e.g. user-controlled) value can hang and crash the process. distribute(0) already returns [], so the positive-integer assumption is only implicit.

Fix

Bound the loop with for (let i = 0; i < count; i++) so it always terminates. A non-positive count now yields an empty distribution (consistent with distribute(0)), and positive-integer counts behave exactly as before (the loop body never referenced count).

Test

Added a test asserting distribute(0) and distribute(-1) return [] instead of looping forever. The full suite passes (63 tests).

distribute(count) used `for (; count !== 0; count--)`, which only reaches
0 for a positive integer count. A negative or fractional count decrements
past 0 forever, allocating currency objects until the process runs out of
memory:

  currency(1).distribute(-1);  // never returns -> heap out of memory
  currency(1).distribute(2.5); // never returns -> heap out of memory

`count` comes straight from the public, number-typed API, so a
caller-supplied count can crash the process. Bound the loop with
`for (let i = 0; i < count; i++)` so it always terminates; a non-positive
count yields an empty distribution, consistent with the existing
distribute(0) behaviour. Positive-integer counts are unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant