[ABRT PATCH] use lchown when chowning files over dbus

[Jiri Moskovcak]

- we don't want to follow symlinks
---
 src/dbus/abrt-dbus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
index c8e0bac..5a58160 100644
--- a/src/dbus/abrt-dbus.c
+++ b/src/dbus/abrt-dbus.c
@@ -604,13 +604,13 @@ static void handle_method_call(GDBusConnection *connection,
             return;
         }
 
-        int chown_res = chown(problem_dir, statbuf.st_uid, pwd->pw_gid);
+        int chown_res = lchown(problem_dir, statbuf.st_uid, pwd->pw_gid);
         dd_init_next_file(dd);
         char *full_name;
         while (chown_res == 0 && dd_get_next_file(dd, /*short_name*/ NULL,
&full_name))
         {
             VERB3 log("chowning %s", full_name);
-            chown_res = chown(full_name, statbuf.st_uid, pwd->pw_gid);
+            chown_res = lchown(full_name, statbuf.st_uid, pwd->pw_gid);
             free(full_name);
         }
 
-- 
1.8.0.1

[Martin Milata]

Looks alright to me.

On Tue, Jan 15, 2013 at 12:57:19 +0100, Jiri Moskovcak wrote:
>  - we don't want to follow symlinks
>  ---
>   src/dbus/abrt-dbus.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>  
>  diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
>  index c8e0bac..5a58160 100644
>  --- a/src/dbus/abrt-dbus.c
>  +++ b/src/dbus/abrt-dbus.c
>  @@ -604,13 +604,13 @@ static void handle_method_call(GDBusConnection *connection,
>               return;
>           }
>   
>  -        int chown_res = chown(problem_dir, statbuf.st_uid, pwd->pw_gid);
>  +        int chown_res = lchown(problem_dir, statbuf.st_uid, pwd->pw_gid);
>           dd_init_next_file(dd);
>           char *full_name;
>           while (chown_res == 0 && dd_get_next_file(dd, /*short_name*/ NULL,
> &full_name))
>           {
>               VERB3 log("chowning %s", full_name);
>  -            chown_res = chown(full_name, statbuf.st_uid, pwd->pw_gid);
>  +            chown_res = lchown(full_name, statbuf.st_uid, pwd->pw_gid);
>               free(full_name);
>           }
>   
>  -- 
>  1.8.0.1
>

[Nikola Pajkovsky]

Martin Milata <mmilata(a)redhat.com&gt; writes:

>  Looks alright to me.
>
>  On Tue, Jan 15, 2013 at 12:57:19 +0100, Jiri Moskovcak wrote:
> > - we don't want to follow symlinks
> > ---
> >  src/dbus/abrt-dbus.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
> > index c8e0bac..5a58160 100644
> > --- a/src/dbus/abrt-dbus.c
> > +++ b/src/dbus/abrt-dbus.c
> > @@ -604,13 +604,13 @@ static void handle_method_call(GDBusConnection *connection,
> >              return;
> >          }
> >  
> > -        int chown_res = chown(problem_dir, statbuf.st_uid, pwd->pw_gid);
> > +        int chown_res = lchown(problem_dir, statbuf.st_uid, pwd->pw_gid);
> >          dd_init_next_file(dd);
> >          char *full_name;
> >          while (chown_res == 0 && dd_get_next_file(dd, /*short_name*/ NULL,
> &full_name))
> >          {
> >              VERB3 log("chowning %s", full_name);
> > -            chown_res = chown(full_name, statbuf.st_uid, pwd->pw_gid);
> > +            chown_res = lchown(full_name, statbuf.st_uid, pwd->pw_gid);
> >              free(full_name);
> >          }
> >  
> > -- 
> > 1.8.0.1
> >  
due to security? hmm?

-- 
Nikola

[Jiri Moskovcak]

On 01/16/2013 08:56 AM, Nikola Pajkovsky wrote:
>  Martin Milata <mmilata(a)redhat.com&gt; writes:
>
> > Looks alright to me.
> >
> > On Tue, Jan 15, 2013 at 12:57:19 +0100, Jiri Moskovcak wrote:
> >> - we don't want to follow symlinks
> >> ---
> >>   src/dbus/abrt-dbus.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
> >> index c8e0bac..5a58160 100644
> >> --- a/src/dbus/abrt-dbus.c
> >> +++ b/src/dbus/abrt-dbus.c
> >> @@ -604,13 +604,13 @@ static void handle_method_call(GDBusConnection
> *connection,
> >>               return;
> >>           }
> >>
> >> -        int chown_res = chown(problem_dir, statbuf.st_uid, pwd->pw_gid);
> >> +        int chown_res = lchown(problem_dir, statbuf.st_uid, pwd->pw_gid);
> >>           dd_init_next_file(dd);
> >>           char *full_name;
> >>           while (chown_res == 0 && dd_get_next_file(dd, /*short_name*/
> NULL, &full_name))
> >>           {
> >>               VERB3 log("chowning %s", full_name);
> >> -            chown_res = chown(full_name, statbuf.st_uid, pwd->pw_gid);
> >> +            chown_res = lchown(full_name, statbuf.st_uid, pwd->pw_gid);
> >>               free(full_name);
> >>           }
> >>
> >> --
> >> 1.8.0.1
> >>
>
>  due to security? hmm?
>  
- yes, yes :)