Re: [PATCH v3 3/4] enter_repo: do not modify input

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 4, 2011 at 7:55 PM, Phil Hord <phil.hord@xxxxxxxxx> wrote:
> On Thu, Sep 29, 2011 at 4:59 PM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote:
>> entr_repo(..., 0) currently modifies the input to strip away
>> trailing slashes. This means that we some times need to copy the
>> input to keep the original.
>
> I'm also modifying enter_repo() so I'm looking a bit closer at this patch now.
>
>> Change it to unconditionally copy it into the used_path buffer so
>> we can safely use the input without having to copy it.
>>
>> Signed-off-by: Erik Faye-Lund <kusmabite@xxxxxxxxx>
>> ---
> [...]
>>  */
>> -char *enter_repo(char *path, int strict)
>> +const char *enter_repo(const char *path, int strict)
>>  {
>>        static char used_path[PATH_MAX];
>>        static char validated_path[PATH_MAX];
>> @@ -297,14 +297,15 @@ char *enter_repo(char *path, int strict)
>>                };
>>                int len = strlen(path);
>>                int i;
>> -               while ((1 < len) && (path[len-1] == '/')) {
>> -                       path[len-1] = 0;
>> +               while ((1 < len) && (path[len-1] == '/'))
>>                        len--;
>> -               }
>> +
>>                if (PATH_MAX <= len)
>>                        return NULL;
>> -               if (path[0] == '~') {
>> -                       char *newpath = expand_user_path(path);
>> +               strncpy(used_path, path, len);
>
> When len < strlen(path), this will will leave used_path unterminated.
>

Good catch, thanks!

>> +
>> +               if (used_path[0] == '~') {
>> +                       char *newpath = expand_user_path(used_path);
>>                        if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
>>                                free(newpath);
>>                                return NULL;
>> @@ -316,24 +317,21 @@ char *enter_repo(char *path, int strict)
>>                         * anyway.
>>                         */
>>                        strcpy(used_path, newpath); free(newpath);
>> -                       strcpy(validated_path, path);
>> -                       path = used_path;
>> +                       strcpy(validated_path, used_path);
>
> The point of 'validated_path' is to keep the original unmolested,
> unexpanded path string (plus DWIM suffix), but here you've just
> replaced validated_path with a copy of the expanded_user_path.  On the
> other hand, we seem always to strcpy(validated_path , path), so we
> might as well get that done up-front.

Yeah, that's probably better.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]