Skip to content

Commit

Permalink
fix(core): error due to integer overflow when there are too many host…
Browse files Browse the repository at this point in the history
… bindings

We currently use 16 bits to store information about nodes in a view.
The 16 bits give us 65536 entries in the array, but the problem is that while
the number is large, it can be reached by ~4300 directive instances with host
bindings which could realistically happen is a very large view, as seen in #37876.
Once we hit the limit, we end up overflowing which eventually leads to a runtime error.

These changes bump to using 20 bits which gives us around 1048576 entries in
the array or 16 times more than the current amount which could still technically
be reached, but is much less likely and the user may start hitting browser limitations
by that point.

I picked the 20 bit number since it gives us enough buffer over the 16 bit one,
while not being as massive as a 24 bit or 32 bit.

I've also added a dev mode assertion so it's easier to track down if it happens
again in the future.

Fixes #37876.
  • Loading branch information
crisbeto committed Jul 11, 2020
1 parent 88d68d2 commit a883497
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
6 changes: 5 additions & 1 deletion packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from '../../me
import {ViewEncapsulation} from '../../metadata/view';
import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization';
import {Sanitizer} from '../../sanitization/sanitizer';
import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertNotEqual, assertNotSame, assertSame} from '../../util/assert';
import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertLessThan, assertNotEqual, assertNotSame, assertSame} from '../../util/assert';
import {createNamedArrayType} from '../../util/named_array_type';
import {initNgDevMode} from '../../util/ng_dev_mode';
import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect';
Expand Down Expand Up @@ -99,6 +99,10 @@ export function setHostBindingsByExecutingExpandoInstructions(tView: TView, lVie
} else {
// If it's not a number, it's a host binding function that needs to be executed.
if (instruction !== null) {
ngDevMode &&
assertLessThan(
currentDirectiveIndex, TNodeProviderIndexes.CptViewProvidersCountShifter,
'Reached the max number of host bindings');
setBindingRootForHostBindings(bindingRootIndex, currentDirectiveIndex);
const hostCtx = lView[currentDirectiveIndex];
instruction(RenderFlags.Update, hostCtx);
Expand Down
13 changes: 7 additions & 6 deletions packages/core/src/render3/interfaces/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,17 @@ export const enum TNodeFlags {
* Corresponds to the TNode.providerIndexes property.
*/
export const enum TNodeProviderIndexes {
/** The index of the first provider on this node is encoded on the least significant bits */
ProvidersStartIndexMask = 0b00000000000000001111111111111111,
/** The index of the first provider on this node is encoded on the least significant bits. */
ProvidersStartIndexMask = 0b00000000000011111111111111111111,

/**
The count of view providers from the component on this node is encoded on the 16 most
significant bits
* The count of view providers from the component on this node is
* encoded on the 20 most significant bits.
*/
CptViewProvidersCountShift = 16,
CptViewProvidersCountShifter = 0b00000000000000010000000000000000,
CptViewProvidersCountShift = 20,
CptViewProvidersCountShifter = 0b00000000000100000000000000000000,
}

/**
* A set of marker values to be used in the attributes arrays. These markers indicate that some
* items are not regular attributes and the processing should be adapted accordingly.
Expand Down

0 comments on commit a883497

Please sign in to comment.