[weston] backends: don't run off the end of strings

Submitted by Derek Foreman on Oct. 23, 2014, 5:21 p.m.

Details

Message ID 1414084913-4140-1-git-send-email-derekf@osg.samsung.com
State Rejected
Headers show

Not browsing as part of any series.

Commit Message

Derek Foreman Oct. 23, 2014, 5:21 p.m.
Strings from the config parser can be of length 0, so we should check
that before checking the string's bytes.

The x11 backend's usage is technically safe since the null terminator
is present, but I've changed it too in case someone's looking at it as
a reference for new code.
---
 src/compositor-wayland.c | 2 +-
 src/compositor-x11.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index bf71a76..e107002 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -2108,7 +2108,7 @@  backend_init(struct wl_display *display, int *argc, char *argv[],
 		if (name == NULL)
 			continue;
 
-		if (name[0] != 'W' || name[1] != 'L') {
+		if (strlen(name) < 2 || name[0] != 'W' || name[1] != 'L') {
 			free(name);
 			continue;
 		}
diff --git a/src/compositor-x11.c b/src/compositor-x11.c
index 1baee29..ae237c8 100644
--- a/src/compositor-x11.c
+++ b/src/compositor-x11.c
@@ -1550,7 +1550,7 @@  x11_compositor_create(struct wl_display *display,
 		if (strcmp(section_name, "output") != 0)
 			continue;
 		weston_config_section_get_string(section, "name", &name, NULL);
-		if (name == NULL || name[0] != 'X') {
+		if (name == NULL || strlen(name) < 1 || name[0] != 'X') {
 			free(name);
 			continue;
 		}

Comments

That won't run off the end of a zero or one-length string.

If strlen() works then there is a nul byte there. The first test against 
'W' will fail because 0 != 'W', and the second test will not be run.

On 10/23/2014 10:21 AM, Derek Foreman wrote:
> Strings from the config parser can be of length 0, so we should check
> that before checking the string's bytes.
>
> The x11 backend's usage is technically safe since the null terminator
> is present, but I've changed it too in case someone's looking at it as
> a reference for new code.
> ---
>   src/compositor-wayland.c | 2 +-
>   src/compositor-x11.c     | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index bf71a76..e107002 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -2108,7 +2108,7 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
>   		if (name == NULL)
>   			continue;
>
> -		if (name[0] != 'W' || name[1] != 'L') {
> +		if (strlen(name) < 2 || name[0] != 'W' || name[1] != 'L') {
>   			free(name);
>   			continue;
>   		}
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 1baee29..ae237c8 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -1550,7 +1550,7 @@ x11_compositor_create(struct wl_display *display,
>   		if (strcmp(section_name, "output") != 0)
>   			continue;
>   		weston_config_section_get_string(section, "name", &name, NULL);
> -		if (name == NULL || name[0] != 'X') {
> +		if (name == NULL || strlen(name) < 1 || name[0] != 'X') {
>   			free(name);
>   			continue;
>   		}
>
Fair enough.  The patch is useless. :)

Thanks

On 23/10/14 01:43 PM, Bill Spitzak wrote:
> That won't run off the end of a zero or one-length string.
> 
> If strlen() works then there is a nul byte there. The first test against
> 'W' will fail because 0 != 'W', and the second test will not be run.
> 
> On 10/23/2014 10:21 AM, Derek Foreman wrote:
>> Strings from the config parser can be of length 0, so we should check
>> that before checking the string's bytes.
>>
>> The x11 backend's usage is technically safe since the null terminator
>> is present, but I've changed it too in case someone's looking at it as
>> a reference for new code.
>> ---
>>   src/compositor-wayland.c | 2 +-
>>   src/compositor-x11.c     | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>> index bf71a76..e107002 100644
>> --- a/src/compositor-wayland.c
>> +++ b/src/compositor-wayland.c
>> @@ -2108,7 +2108,7 @@ backend_init(struct wl_display *display, int
>> *argc, char *argv[],
>>           if (name == NULL)
>>               continue;
>>
>> -        if (name[0] != 'W' || name[1] != 'L') {
>> +        if (strlen(name) < 2 || name[0] != 'W' || name[1] != 'L') {
>>               free(name);
>>               continue;
>>           }
>> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
>> index 1baee29..ae237c8 100644
>> --- a/src/compositor-x11.c
>> +++ b/src/compositor-x11.c
>> @@ -1550,7 +1550,7 @@ x11_compositor_create(struct wl_display *display,
>>           if (strcmp(section_name, "output") != 0)
>>               continue;
>>           weston_config_section_get_string(section, "name", &name, NULL);
>> -        if (name == NULL || name[0] != 'X') {
>> +        if (name == NULL || strlen(name) < 1 || name[0] != 'X') {
>>               free(name);
>>               continue;
>>           }
>>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel