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: prevent buffer overflow #74

Merged
merged 2 commits into from
Aug 27, 2020
Merged

fix: prevent buffer overflow #74

merged 2 commits into from
Aug 27, 2020

Conversation

greut
Copy link
Member

@greut greut commented Aug 26, 2020

There is probably a better way though.

Closes #73

Closes #73

Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
@xuhdev
Copy link
Member

xuhdev commented Aug 26, 2020

The test max_property_name is broken:

[00:03:37] test 160
[00:03:37]         Start 160: max_property_name
[00:03:37] 
[00:03:37] 160: Test command: C:\projects\editorconfig-core-c\bin\x64-static\core\bin\Release\editorconfig.exe "-f" "limits.in" "C:/projects/editorconfig-core-c/tests/parser/test1"
[00:03:37] 160: Test timeout computed to be: 10000000
[00:03:37] 160: 00000000000000000000000000000000000000000000000001��n�=50
[00:03:37] 160/189 Test #160: max_property_name .........................***Failed  Required regular expression not found. Regex=[^00000000000000000000000000000000000000000000000001=50[ 	
[00:03:37] 
[00:03:37] ]*$
[00:03:37] ]  0.01 sec
[00:03:37] 00000000000000000000000000000000000000000000000001��n�=50

Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
@Vogtinator
Copy link

Why not just use a dynamically allocated string here? Currently it silently truncates and

somelong...name = bar
somelong...namefoo = foo

Would end up parsed as somelongname=foo.

@greut
Copy link
Member Author

greut commented Aug 27, 2020

Why not just use a dynamically allocated string here?

It would make sense to already implement that proposed change, indeed. editorconfig/editorconfig-core-test#41

However, the goal here is to avoid the buffer overflow.

@Vogtinator
Copy link

Actually, looking at the code the allocation can actually be avoided at that point - just use strcasecmp in find_name_value_from_name. Later, set_name_value does a strdup anyway, so that could take care of strlwr as well.

@@ -138,7 +138,7 @@ static int array_editorconfig_name_value_add(
int name_value_pos;
/* always use name_lwr but not name, since property names are case
* insensitive */
char name_lwr[MAX_PROPERTY_NAME];
char name_lwr[MAX_PROPERTY_NAME+1] = {0};
Copy link
Member

Choose a reason for hiding this comment

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

This is a point that I missed in the original implementation. Thanks!

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.

Segfault while reading .editorconfig generated by JetBrains IDE
3 participants