Skip to content
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

Fix read past the end of a buffer in pool.c. #2139

Merged
merged 2 commits into from
Aug 5, 2017

Conversation

chalcolith
Copy link
Member

This change fixes ponyint_pool_realloc_size, which was reading past the end of the old buffer, which in some circumstances triggered a page fault.

This change fixes `ponyint_pool_realloc_size`, which was reading past the end of the old buffer, which in some circumstances triggered a page fault.
@@ -971,7 +971,7 @@ void* ponyint_pool_realloc_size(size_t old_size, size_t new_size, void* p)
new_p = pool_alloc_size(new_adj_size);
}

memcpy(new_p, p, new_size);
memcpy(new_p, p, old_size);
Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

But is it guaranteed that old_size < new_size here?

If not, wouldn't we want to take min(old_size, new_size) as the copy length?

Copy link
Member Author

Choose a reason for hiding this comment

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

D'oh, of course.

@SeanTAllen
Copy link
Member

I'm going to go out on a limb and say we should do a 0.17.1 for this. Thoughts?

@jemc
Copy link
Member

jemc commented Aug 5, 2017

So far it looks like this function is only used in the compiler, not in the runtime. Is that correct, @kulibali and @Praetonus?

If that's correct, I wouldn't necessarily consider this to be a release-triggering fix.

@chalcolith
Copy link
Member Author

It looks like it is only used in libponyc\ast\lexer.c.

@SeanTAllen
Copy link
Member

ok, no release then! good. i did want to go through that again today.

@chalcolith chalcolith added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Aug 5, 2017
@chalcolith
Copy link
Member Author

Is this changelog-worthy?

@chalcolith chalcolith merged commit 7da6afe into ponylang:master Aug 5, 2017
ponylang-main added a commit that referenced this pull request Aug 5, 2017
@chalcolith chalcolith deleted the buffer_overread branch August 8, 2017 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants