Skip to content

Fix count variable calculation in typedarray copyWithin #3158

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

Merged
merged 1 commit into from
Oct 7, 2019

Conversation

szilagyiadam
Copy link
Contributor

@szilagyiadam szilagyiadam commented Sep 24, 2019

Fixes #3130

JerryScript-DCO-1.0-Signed-off-by: Adam Szilagyi aszilagy@inf.u-szeged.hu

@rerobika rerobika added bug Undesired behaviour ES2015 Related to ES2015 features labels Sep 24, 2019
Copy link
Member

@rerobika rerobika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@dbatyai dbatyai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the correct fix for the issue, the problem is not the size of the allocation. The issue is that the number of elements that should be copied is incorrectly calculated in the copyWithin method. All of these variables should be uint32_t and should be moved into the else block, where the bounds are checked.

@szilagyiadam szilagyiadam force-pushed the fix_3130 branch 2 times, most recently from 3d3409a to 91af0fd Compare September 30, 2019 12:06
@szilagyiadam szilagyiadam changed the title Limit arraybuffer allocation size Fix count variable calculation in typedarray copyWithin Sep 30, 2019
@szilagyiadam szilagyiadam force-pushed the fix_3130 branch 3 times, most recently from 3c46d52 to 5bb3f0f Compare September 30, 2019 12:46
Copy link
Member

@dbatyai dbatyai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please rebase.

Fixes jerryscript-project#3130

JerryScript-DCO-1.0-Signed-off-by: Adam Szilagyi aszilagy@inf.u-szeged.hu
Copy link
Member

@rerobika rerobika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the related regression tests as well.

@szilagyiadam
Copy link
Contributor Author

I cant put the test case anywhere because if i put it into the fails folder, it passes beacuse of the system allocator, and if i put it into the es2015 folder, it fails, beacuse we dont have enough memory.

@rerobika
Copy link
Member

rerobika commented Oct 7, 2019

I see, now it's ready to go.

Copy link
Member

@rerobika rerobika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rerobika rerobika merged commit 0f754ff into jerryscript-project:master Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Undesired behaviour ES2015 Related to ES2015 features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

heap-buffer-overflow in ecma_builtin_typedarray_prototype_copy_within
3 participants