On Sun, Feb 12, 2023 at 09:16:58PM +0000, Job Snijders wrote:
> I noticed there is an issue in openrsync when a port is specified in the
> rsync:// URL, the port number ends up becoming part of the path:
>
> $ openrsync -r rsync://rsync.roa.tohunet.com:3873/repo/ /tmp/r
> rsync: [sender] change_dir "/3873/repo" (in repo) failed: No such file or
> directory (2)
>
> The below changeset seems to improve the situation, but might not be the
> best way to fix it. I found fargs_parse() a bit hard to understand.
No kidding...
> OK? Suggestions?
>
> Kind regards,
>
> Job
>
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/rsync/main.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 main.c
> --- main.c 2 Aug 2022 20:01:12 -0000 1.65
> +++ main.c 12 Feb 2023 21:06:00 -0000
> @@ -233,8 +233,12 @@ fargs_parse(size_t argc, char *argv[], s
> strncasecmp(cp, "rsync://", 8) == 0) {
> /* rsync://path */
> cp += 8;
> - if ((ccp = strchr(cp, ':'))) /* skip :port */
> + /* skip :port */
> + if ((ccp = strchr(cp, ':')) != NULL) {
> *ccp = '\0';
> + while (cp[len] != '/' && len < j)
> + len++;
> + }
There are various issues here. First, you don't know that cp[len] is in
any way related to the location of ':' in the string since the strncmp()
only happens later.
This is equivalent to your while loop but starting at what we know to be
after the colon:
len += strcspn(ccp + 1, "/") + 1;
This brings me to the next problem: we must not modify len, as it is
the length of f->host. If there are multiple sources, len my grow and
eventually cp[len] could access out of bounds.
While it is not super clean, the easiest fix for this would be to do
/* rsync://path[:host] */
size_t save_len = len;
after the strncasecmp() above, then after the memmove() at the end of the
scope reset the length to what it originally was:
len = save_len;
A bit cleaner would be to replace uses of len with
/* rsync://path[:host] */
size_t module_offset = len;
...
module_offset += strcspn(ccp + 1, "/") + 1;
then replace all uses of len in this scope *after* the strncmp below with
module_offset.
> if (strncmp(cp, f->host, len) ||
> (cp[len] != '/' && cp[len] != '\0'))
> errx(ERR_SYNTAX, "different remote host: %s",
>
