Procedures named with Object prototype attr names cause crash upon executing

[apple502j]

Expected Behavior

Scratch should not crash, especially in the thread (because it does not show the blue screen and consume memory by spamming browser console)

Actual Behavior

It crashes

Steps to Reproduce

Make a procedure named "hasOwnProperty" or something else in the object prototype.

Operating System and Browser

any

Fixes

Procedure related cache attributes need to use prototypeless object or Map.

This is different from #2332 because modifying json is not necessary for this to happen.

[aetinx]

When an issue has both "High severity" and "Low impact" tags. LOL.

[benjiwheeler]

When an issue has both "High severity" and "Low impact" tags. LOL.

This combination means "when it happens it's bad, but it seldom happens".

[benjiwheeler]

@apple502j Do you have repro steps? I don't totally understand what the problem is.

[adroitwhiz]

Create a custom block named hasOwnProperty, then run it:

The VM will silently crash and the console will become filled with errors.

[AmazingMech2418]

Can reproduce.

Judging by the errors thrown, it is an internal React error dealing with how this code processes custom blocks. Seems like something might use the name of the custom block with either subscript or dot notation to get a property from an object, and since the property is in the Object prototype, it would already be defined and might throw an error. Clearly not method pollution though given that isPrototypeOf is never used in the code, yet still throws the error.

isPrototypeOf is also never used in the React source code either.

[apple502j]

Wrong analysis; this is probably VM cache misbehaving, e.g. https://github.com/LLK/scratch-vm/blob/develop/src/engine/blocks.js#L228

[AmazingMech2418]

Wrong analysis; this is probably VM cache misbehaving, e.g. https://github.com/LLK/scratch-vm/blob/develop/src/engine/blocks.js#L228

That's the same line I found the error to be in. It uses subscript notation with the key as the name of the custom block, so if it is in the Object prototype, it might throw an error.

[AmazingMech2418]

Actually, sorry, it was https://github.com/LLK/scratch-vm/blob/607cd2d392b0d16eb14e3512de9ac6741f2dfe3d/src/engine/blocks.js#L264 where I found it to be.