Skip to content

Fixed heap corruption, extra byte for NULL termination symbol #8

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
Apr 30, 2017

Conversation

kykc
Copy link

@kykc kykc commented Apr 30, 2017

As stated here in general case we need to allocate N+1 bytes for string of length N in order to copy over NULL termination byte, otherwise we'll corrupt heap memory with strcpy call.

So, in this case we need to allocate strlen(color_str) bytes, as we're omitting first symbol.

Testcase which I used to test if I'm right about this:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

int main()
{
	const char* test = "[255;255;255]";

	printf("string length: %i\n", (int)strlen(test));

	char* test_dest = malloc(strlen(test) + 1);

	test_dest[strlen(test)] = 0xFF;

	printf("value before strcpy: %hhx\n", test_dest[strlen(test)]);

	strcpy(test_dest, test);

	printf("value after strcpy: %hhx\n", test_dest[strlen(test)]);

	free(test_dest);

	return 0;
}

@Gibtnix Gibtnix merged commit 330aa1b into Gibtnix:master Apr 30, 2017
@Gibtnix
Copy link
Owner

Gibtnix commented Apr 30, 2017

Thanks for the PR, I just merged it; however I'll also set the last byte explicitly to 0 (cf. main.c), otherwise I think it might be possible that it is initialized with something different than 0.

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.

2 participants