Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH
Tomi Ollilawrites: > > The probability for user error is pretty small there -- if there is > typo/thinko there things usually just starts failing. Security is > easier to break elsewhere than here (e.g. borken PATH...) > > I'd keep the current implementation of test_for_executable()... > > Tomi > OK ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH
On Fri, Aug 12 2016, Daniel Kahn Gillmorwrote: > On Fri 2016-08-12 03:38:53 -0400, David Bremner wrote: >> Daniel Kahn Gillmor writes: >> Should we distinguish between relative and absolute paths here? I can't think of any security implications, but I'm wondering if a relative path is likely just a user error. >>> >>> I don't think a relative path is necessarily a user error. I certainly >>> use relative paths myself from time to time. >> >> As configuration values? That seems quite fragile, since it depends on >> the current working directory when notmuch is run. > > rarely! but sometimes i do it because i'm testing things in strange > ways, and it can be a bit frustrating to have a tool second-guess me > when it seems like i ought to be able to drop the same string i'm using > on the command line into the configuration. > > I don't feel strongly, though. if you want to say that bare words found > in the $PATH and absolute filenames (starting with /) are fine in the > notmuch config but relative paths are not, i'd be ok with that :) From consistency point of view, current patch not checking it being absolute might prevail -- I don't see database.path being checked for being absolute... The probability for user error is pretty small there -- if there is typo/thinko there things usually just starts failing. Security is easier to break elsewhere than here (e.g. borken PATH...) I'd keep the current implementation of test_for_executable()... Tomi > > --dkg ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH
On Fri 2016-08-12 03:38:53 -0400, David Bremner wrote: > Daniel Kahn Gillmorwrites: > >>> Should we distinguish between relative and absolute paths here? I can't >>> think of any security implications, but I'm wondering if a relative path >>> is likely just a user error. >> >> I don't think a relative path is necessarily a user error. I certainly >> use relative paths myself from time to time. > > As configuration values? That seems quite fragile, since it depends on > the current working directory when notmuch is run. rarely! but sometimes i do it because i'm testing things in strange ways, and it can be a bit frustrating to have a tool second-guess me when it seems like i ought to be able to drop the same string i'm using on the command line into the configuration. I don't feel strongly, though. if you want to say that bare words found in the $PATH and absolute filenames (starting with /) are fine in the notmuch config but relative paths are not, i'd be ok with that :) --dkg ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH
Daniel Kahn Gillmorwrites: >> >> Should we distinguish between relative and absolute paths here? I can't >> think of any security implications, but I'm wondering if a relative path >> is likely just a user error. > > I don't think a relative path is necessarily a user error. I certainly > use relative paths myself from time to time. As configuration values? That seems quite fragile, since it depends on the current working directory when notmuch is run. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH
On Fri 2016-08-12 01:51:16 -0400, David Bremner wrote: > Daniel Kahn Gillmorwrites: > >> This is a utility function we can use to see whether an executa> >> +if (strchr (exename, '/')) { >> +if (0 == access (exename, X_OK)) >> +return TRUE; >> +else >> +return FALSE; >> +} > > Should we distinguish between relative and absolute paths here? I can't > think of any security implications, but I'm wondering if a relative path > is likely just a user error. I don't think a relative path is necessarily a user error. I certainly use relative paths myself from time to time. >> +path = (char *) malloc (n); >> +if (! path) >> +return FALSE; > > I kindof hate hiding the error here, although I agree it's > unlikely. What about the unixy return 0 ok, 1 not found -1 error? > >> +confstr (_CS_PATH, path, n); >> +} >> + >> +tok = strtok_r (path, ":", ); >> +while (tok) { > > I guess it's fine to modify path here, but another option is > strtok_len (in string-util.h) I'm ok with both of these changes -- do you want to propose a variant for this patch? thanks for going through and trying to get this stuff building again. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH
Daniel Kahn Gillmorwrites: > This is a utility function we can use to see whether an executa> > +if (strchr (exename, '/')) { > + if (0 == access (exename, X_OK)) > + return TRUE; > + else > + return FALSE; > +} Should we distinguish between relative and absolute paths here? I can't think of any security implications, but I'm wondering if a relative path is likely just a user error. > + path = (char *) malloc (n); > + if (! path) > + return FALSE; I kindof hate hiding the error here, although I agree it's unlikely. What about the unixy return 0 ok, 1 not found -1 error? > + confstr (_CS_PATH, path, n); > +} > + > +tok = strtok_r (path, ":", ); > +while (tok) { I guess it's fine to modify path here, but another option is strtok_len (in string-util.h) ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch