[Labs-l] [tools] New version of take

[Platonides]

On 22/06/13 20:24, Petr Bena wrote:
> resolved.

No, you're still vulnerable (1609de5f).
Moreover, your are changing a file different than the one you checked.

I also noticed that you are closing each descriptor thrice.

One instance is trivially fixed by:
> diff --git a/include/Take.h b/include/Take.h
> index dd063f3..4fa2a41 100644
> --- a/include/Take.h
> +++ b/include/Take.h
> @@ -20,7 +20,7 @@ class Take
>          int fd;
>          FD(int f): fd(f)                { };
>          ~FD()                           { Close(); };
> -        void Close()                    { if(fd >= 0) close(fd); };
> +        void Close()                    { if(fd >= 0) close(fd); fd = -1; };
>          operator int (void) const       { return fd; };
>      };


The other close was hidden in copied objects (why did you need an 
object? This would have been much easier simply passing the fds!)

> diff --git a/include/Take.h b/include/Take.h
> index dd063f3..ac8a417 100644
> --- a/include/Take.h
> +++ b/include/Take.h
> @@ -29,10 +29,10 @@ class Take
>          virtual ~Take();
>      protected:
>      private:
> -        bool Overtake(string path, FD fd);
> +        bool Overtake(string path, FD& fd);
>          bool Verify(string path);
>          static int Callback(const char* path, const struct stat *sb, int typeflag, struct FTW *ftwbuf);
> -        static void ChangeOwner(string path, uid_t owner, FD fd, bool ChangeGroup);
> +        static void ChangeOwner(string path, uid_t owner, FD& fd, bool ChangeGroup);
>          static bool CheckGroups(struct stat info);
>          static bool CheckHL(string path);
>          static bool CheckHL(struct stat info);
> diff --git a/src/Take.cpp b/src/Take.cpp
> index 8a111d6..accfc02 100644
> --- a/src/Take.cpp
> +++ b/src/Take.cpp
> @@ -126,7 +126,7 @@ int Take::Callback(const char* path, const struct stat *sb, int typeflag, struct
>      return 0;
>  }
>
> -void Take::ChangeOwner(string path, uid_t owner, Take::FD fd, bool ChangeGroup)
> +void Take::ChangeOwner(string path, uid_t owner, Take::FD& fd, bool ChangeGroup)
>  {
>      if (!ChangeGroup)
>      {
> @@ -215,7 +215,7 @@ bool Take::CheckHL(string path)
>      return false;
>  }
>
> -bool Take::Overtake(string path, Take::FD fd)
> +bool Take::Overtake(string path, Take::FD& fd)
>  {
>      char * p = realpath(path.c_str(), NULL);
>