On Wed, Jun 27, 2018 at 11:15:38AM -0600, Theo de Raadt wrote:
> > Index: misc.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/ssh/misc.c,v
> > retrieving revision 1.129
> > diff -u -p -u -r1.129 misc.c
> > --- misc.c 9 Jun 2018 03:01:12 -0000 1.129
> > +++ misc.c 26 Jun 2018 08:42:34 -0000
> > @@ -521,8 +521,13 @@ colon(char *cp)
> > flag = 1;
> > if (*cp == ']' && *(cp+1) == ':' && flag)
> > return (cp+1);
> > - if (*cp == ':' && !flag)
> > - return (cp);
> > + if (*cp == ':' && !flag) {
> > + /* check for :skey@ addition */
> > + if (*(cp+1) == 's' && *(cp+5) =='@')
> > + continue;
> > + else
> > + return (cp);
>
> That diff is crazy and wrong.
>
> First off, the string could be less than 5 long, so you are
> reaching beyond the end, example:
>
> scp pjp:s .
>
> Secondly, :skey is only one form of 4.4BSD/OpenBSD login service.
> Some of them are listed in the AUTHENTICATION section of login.conf(5)
>
> Thirdly, it becomes impossible to copy certain filenames that contain
> an :, therefore this is no longer backwards compatible and may affect
> other use cases, which may even exist in the wild today.
>
> Fourth: such string seperation will also impacts non-OpenBSD systems
> with different authentication support, because the change will land
> in OpenSSH-portable.
>
> I don't think this is the way to handle the problem.
Thanks for taking the time, I really didn't have time but I took some aside
to come up with another idea. It will have errors if the file being
transferred has a @ in it, I think. Feel free to improve on it, or tell me
to give it up.
Cheers,
-peter
Index: misc.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/misc.c,v
retrieving revision 1.129
diff -u -p -u -r1.129 misc.c
--- misc.c 9 Jun 2018 03:01:12 -0000 1.129
+++ misc.c 27 Jun 2018 19:42:32 -0000
@@ -510,6 +510,27 @@ char *
colon(char *cp)
{
int flag = 0;
+ char save;
+ char *endmethod = NULL;
+
+ struct svcs {
+ char *service;
+ } *psvcs;
+
+ struct svcs services[] = {
+ { "activ" },
+ { "chpass"},
+ { "crypto"},
+ { "lchpass"},
+ { "passwd"},
+ { "radius"},
+ { "reject"},
+ { "skey"},
+ { "snk"},
+ { "token"},
+ { "yubikey"},
+ { NULL} };
+
if (*cp == ':') /* Leading colon is part of file name. */
return NULL;
@@ -521,8 +542,27 @@ colon(char *cp)
flag = 1;
if (*cp == ']' && *(cp+1) == ':' && flag)
return (cp+1);
- if (*cp == ':' && !flag)
+ if (*cp == ':' && !flag) {
+ if ((endmethod = strchr(cp, '@')) != NULL) {
+ save = *endmethod;
+ *endmethod = '\0';
+ cp++;
+
+ for (psvcs = &services[0]; *psvcs->service;
psvcs++) {
+ if (strcmp(cp, psvcs->service) == 0)
+ break;
+ }
+
+ *endmethod = save;
+
+ if (psvcs->service) {
+ cp = endmethod + 1;
+ continue;
+ }
+ }
+
return (cp);
+ }
if (*cp == '/')
return NULL;
}
