event-target-shim icon indicating copy to clipboard operation
event-target-shim copied to clipboard

`EventTarget.removeEventListener` called from within listener callback results in index out of bounds access

Open aplr opened this issue 11 months ago • 0 comments

If EventTarget.removeEventListener is called from within the listeners callback, this may result in an access to an array index which does not exist any more when calling listeners in a loop:

https://github.com/mysticatea/event-target-shim/blob/a37993c95ed9cc1d7cb89358ea126ed4f95372e8/src/lib/event-target.ts#L243-L267

As this library is used as a ponyfill in vercel's edge-runtime this becomes evident when using libraries which rely on removing event listeners in the listener callback. One instance of this case is documented in the following issue:

https://github.com/bufbuild/connect-es/issues/749

Insights

When calling dispatchEvent, the list of listeners consists of two items:

listeners = [
  { callback: [function onAbort], flags: 0 },
  { callback: [function onAbort], flags: 4 }
]

In the first iteration, the callback removes the first listener from list.listeners. However, the local, deconstructed listeners still points to the old list with 2 items. This becomes a problem in the second iteration, as it's a "once" listener, which the code tries to remove from list.listeners. As the list only contains one listener, the operation fails as it's working on a undefined value.

Possible solution

I don't know if this case should work, or this library is still maintained. However, it's used by nextjs and I'm surprised it did not become evident before.

I did not dig too deep, but one thing I've noticed is when always referencing the listeners list by list.listeners the issue does not arise. However, I don't know if this messes up other parts of the listener invoke logic.

aplr avatar Aug 09 '23 14:08 aplr