[BUG][PATCH][BTRFS-PROGS] Bug overflow fix

[BUG][PATCH][BTRFS-PROGS] Bug overflow fix

[Rock Lee]

If there's is a long name directory exists in the /dev, then an
overflow will hit in function utils.c btrfs_scan_one_dir:1013!

The minimal fix is to use snprintf instead of strcpy.

The reason why not using strncpy is that, if there is no null byte
among the first n bytes of src, the string  placed  in  dest
will not be null - terminated.

Signed-off-by: Rock Lee <zimilo@code-trick.com>
---
 utils.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/utils.c b/utils.c
index 3c88d2e..7200aef 100644
--- a/utils.c
+++ b/utils.c
@@ -969,7 +969,7 @@ int btrfs_scan_one_dir(char *dirname, int run_ioctl)
 	pending = malloc(sizeof(*pending));
 	if (!pending)
 		return -ENOMEM;
-	strcpy(pending->name, dirname);
+	snprintf(pending->name, sizeof(pending->name), "%s", dirname);

 again:
 	dirname_len = strlen(pending->name);
@@ -1010,7 +1010,8 @@ again:
 				ret = -ENOMEM;
 				goto fail;
 			}
-			strcpy(next->name, fullpath);
+			snprintf(next->name, sizeof(next->name),
+				 "%s", fullpath);
 			list_add_tail(&next->list, &pending_list);
 		}
 		if (!S_ISBLK(st.st_mode)) {
-- 
1.7.7.6

Re: [BUG][PATCH][BTRFS-PROGS] Bug overflow fix

[David Sterba]

On Wed, Oct 24, 2012 at 04:24:02PM +0800, Rock Lee wrote:
> If there's is a long name directory exists in the /dev, then an
> overflow will hit in function utils.c btrfs_scan_one_dir:1013!
> 
> The minimal fix is to use snprintf instead of strcpy.
> 
> The reason why not using strncpy is that, if there is no null byte
> among the first n bytes of src, the string  placed  in  dest
> will not be null - terminated.
> 
> ---
> index 3c88d2e..7200aef 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -969,7 +969,7 @@ int btrfs_scan_one_dir(char *dirname, int run_ioctl)
>  	pending = malloc(sizeof(*pending));
>  	if (!pending)
>  		return -ENOMEM;
> -	strcpy(pending->name, dirname);
> +	snprintf(pending->name, sizeof(pending->name), "%s", dirname);

pending is defined as

919 struct pending_dir {
920         struct list_head list;
921         char name[256];
922 };

and name is supposed to hold a full path, so it should be of PATH_MAX
(4096) size, 256 is a limit for a single filename.

There's another hardcoded value for a path

971         dirname_len = strlen(pending->name);
972         pathlen = 1024;
^^^
973         fullpath = malloc(pathlen);
974         dirname = pending->name;

that shuld be of PATH_MAX size.

thanks,
david

Re: [BUG][PATCH][BTRFS-PROGS] Bug overflow fix

[Rock Lee]

Yeah, I will improve the patch to get it better.

Thanks,
- Rock

2012/10/25 David Sterba <dave@jikos.cz>:
> On Wed, Oct 24, 2012 at 04:24:02PM +0800, Rock Lee wrote:
>> If there's is a long name directory exists in the /dev, then an
>> overflow will hit in function utils.c btrfs_scan_one_dir:1013!
>>
>> The minimal fix is to use snprintf instead of strcpy.
>>
>> The reason why not using strncpy is that, if there is no null byte
>> among the first n bytes of src, the string  placed  in  dest
>> will not be null - terminated.
>>
>> ---
>> index 3c88d2e..7200aef 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -969,7 +969,7 @@ int btrfs_scan_one_dir(char *dirname, int run_ioctl)
>>       pending = malloc(sizeof(*pending));
>>       if (!pending)
>>               return -ENOMEM;
>> -     strcpy(pending->name, dirname);
>> +     snprintf(pending->name, sizeof(pending->name), "%s", dirname);
>
> pending is defined as
>
> 919 struct pending_dir {
> 920         struct list_head list;
> 921         char name[256];
> 922 };
>
> and name is supposed to hold a full path, so it should be of PATH_MAX
> (4096) size, 256 is a limit for a single filename.
>
> There's another hardcoded value for a path
>
> 971         dirname_len = strlen(pending->name);
> 972         pathlen = 1024;
> ^^^
> 973         fullpath = malloc(pathlen);
> 974         dirname = pending->name;
>
> that shuld be of PATH_MAX size.
>
> thanks,
> david