Skip to content

Patch to guard against integer overflow#19

Closed
ingydotnet wants to merge 1 commit into
masterfrom
integer-overflow
Closed

Patch to guard against integer overflow#19
ingydotnet wants to merge 1 commit into
masterfrom
integer-overflow

Conversation

@ingydotnet
Copy link
Copy Markdown
Member

Originally from Ruby binding.

Comment thread src/api.c
YAML_DECLARE(int)
yaml_stack_extend(void **start, void **top, void **end)
{
void *new_start = yaml_realloc(*start, ((char *)*end - (char *)*start)*2);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we sticking to C89? What are our supported operating systems if we're deciding to stick to that?

Comment thread src/api.c
void *new_start = yaml_realloc(*start, ((char *)*end - (char *)*start)*2);
void *new_start;

if ((char *)*end - (char *)*start >= INT_MAX / 2)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason not to calculate

(char *)*end - (char *)*start

Once and use it here and below? I know it shouldn't change very much, but I feel like it'll be cleaner, especially if we give it a very clear name.

@ingydotnet
Copy link
Copy Markdown
Member Author

This was originally from Florian Weimer fweimer@redhat.com circa Feb 2014.

Would be good to add a test. Might want to find/ask Florian.

@sigmavirus24
Copy link
Copy Markdown
Contributor

Going to close and reopen this to trigger AppVeyor

@sigmavirus24 sigmavirus24 reopened this Dec 6, 2016
sigmavirus24 pushed a commit to sigmavirus24/libyaml that referenced this pull request Apr 6, 2017
sigmavirus24 pushed a commit to sigmavirus24/libyaml that referenced this pull request Apr 6, 2017
It is very unfortunate, but we have to copy the file. I'll think
how it is possible to restructure our images to avoid duplication.

Issue yaml#19
sigmavirus24 pushed a commit to sigmavirus24/libyaml that referenced this pull request Apr 6, 2017
@ingydotnet ingydotnet mentioned this pull request Jan 7, 2018
Copy link
Copy Markdown

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

This commit got landed in #125. I believe this PR can be closed.

@ingydotnet
Copy link
Copy Markdown
Member Author

@dtolnay thanks, closing.

@ingydotnet ingydotnet closed this Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants