bug-inetutils
[Top][All Lists]
Advanced

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

Re: [bug-inetutils] [PULL-REQUEST][PATCH] src/rcp.c: fix couldn't copy s


From: Zhixiong Chi
Subject: Re: [bug-inetutils] [PULL-REQUEST][PATCH] src/rcp.c: fix couldn't copy subdirectory issue
Date: Thu, 30 May 2019 18:01:37 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0

Hi All,

I couldn't find the way how to contribute patch in the website.

Could you help review it?

Thanks.


On 2019年05月30日 18:00, Zhixiong Chi wrote:
The namebuf will get the same allocation address as the one before
free operation, at the same time because of the recursive call for
sink function, the targ and namebuf point the same address, then it
cause the namebuf will get the wrong value with the snprintf function.
Since the snprintf function doesn't like the strcpy function which
can overwrite the destination.
eg:
  char tmp[20] = "test";
  snprintf(tmp,20,"%s%s",tmp,"yes");
The result of tmp -> yes other than "testyes". It will cause the wrong value.
So we couldn't free at this place before allocation(since recursive call).

The sink function flow is as follows:
  >sink(int argc, char*argv[])
  >{
  > ...
  > targ = *argv;
  > if (targisdir)
  > {
  >   static char *namebuf = NULL;
  >   free (namebuf);
  >   namebuf = malloc (need);
  >   snprintf (namebuf, cursize, "%s%s%s", targ, *targ ? "/" : "", cp); -> 
issue
  > }
  > np = namebuf;
  > if (buf[0] == "D")
  > {
  >   vect[0] = np;
  >   sink (1, vect);
  > }
  > ...
  >}

I move the free operation into the end of every loop to avoid the memory leak,
due to vect = np = namebuf, the np always used until the end of loop.
At the same time because of the scope of namebuf, we free np instead of namebuf
in the loop to avoid the compiling failure if the targisdir is true.

Testcase:
# cd /root
# rcp address@hidden:/test/test1 ./
# ls ./test
Result: Only the test directory had been copied. All of the subdirectory
hadn't been copied. At the same time since the snprintf error test1
will be copyied into / directory.

Signed-off-by: Zhixiong Chi <address@hidden>
---
  src/rcp.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/rcp.c b/src/rcp.c
index f1a12f04..9357eae5 100644
--- a/src/rcp.c
+++ b/src/rcp.c
@@ -995,7 +995,6 @@ sink (int argc, char *argv[])
          need = strlen (targ) + strlen (cp) + 250;
          if (need > cursize)
            {
-             free (namebuf);
              namebuf = malloc (need);
              if (namebuf)
                cursize = need;
@@ -1162,6 +1161,8 @@ sink (int argc, char *argv[])
        case DISPLAYED:
          break;
        }
+      if (targisdir && np)
+       free(np);
      }
  screwup:
    run_err ("protocol error: %s", why);

--
---------------------
Thanks,
Zhixiong Chi
Tel: +86-10-8477-7036




reply via email to

[Prev in Thread] Current Thread [Next in Thread]