Re: [bugfix] xterm(1) needs "cpath" pledge(2)

2018-07-28 Thread Theo de Raadt
I didn't realize you were using coloured cursors.

You make me sad.  You don't know what the bug is.  You don't know how to
reproduce it.  You don't have a plausible call path.  You grep, and yet
don't dig deeper to see if that code you find is initialization phase or
activated later during runtime after the pledge call.

Essentially, you don't attempt any of the processes we call
"development", but hey, here's a diff...

Your answer when encountering pledge adversity, to add all the required
pledges, and not look deeper.

As I say, it makes me sad.

This should be looked into deeper, and we may decide that whatever thing
xterm is doing here should be disabled.

Leonid Bobrov  wrote:
> Like I said yesterday, I don't know how to reproduce this bug, it just
> happened to me and I got this dmesg(1):
> xterm[90461]: pledge "cpath", syscall 5
> 
> Right now I quickly grep(1)'ed xterm(1)'s source code:
> mazocomp$ egrep "mkdir\(|unlink\(|rmdir\(" -R . -n
> ./misc.c:760:   && mkdir(filename, 0700) == 0) {
> ./misc.c:804:   unlink(xterm_cursor_theme);
> ./misc.c:805:   rmdir(my_path);
> 
> The diff below should fix it:
> 
> Index: main.c
> ===
> RCS file: /cvs/xenocara/app/xterm/main.c,v
> retrieving revision 1.43
> diff -u -p -u -p -r1.43 main.c
> --- main.c29 Mar 2018 20:22:05 -  1.43
> +++ main.c28 Jul 2018 18:48:15 -
> @@ -2782,12 +2782,12 @@ main(int argc, char *argv[]ENVP_ARG)
>  if (data &&
>  (strstr(data, "exec-formatted") || strstr(data, 
> "exec-selectable"))) {
>  
> -if (pledge("stdio rpath wpath id proc exec tty", NULL) == -1) {
> +if (pledge("stdio rpath cpath wpath id proc exec tty", NULL) == 
> -1) {
>  xtermWarning("pledge\n");
>  exit(1);
>  }
>  } else {
> -if (pledge("stdio rpath wpath id proc tty", NULL) == -1) {
> +if (pledge("stdio rpath cpath wpath id proc tty", NULL) == -1) {
> xtermWarning("pledge\n");
> exit(1);
> }
> 



Re: [bugfix] xterm(1) needs "cpath" pledge(2)

2018-07-28 Thread Sebastien Marie
Hi,

First, thanks to (trying) to search for solve the problem you encountered.

On Sat, Jul 28, 2018 at 10:08:04PM +0300, Leonid Bobrov wrote:
> Hi!
> 
> Like I said yesterday, I don't know how to reproduce this bug, it just
> happened to me and I got this dmesg(1):
> xterm[90461]: pledge "cpath", syscall 5
> 
> Right now I quickly grep(1)'ed xterm(1)'s source code:
> mazocomp$ egrep "mkdir\(|unlink\(|rmdir\(" -R . -n
> ./misc.c:760:   && mkdir(filename, 0700) == 0) {
> ./misc.c:804:   unlink(xterm_cursor_theme);
> ./misc.c:805:   rmdir(my_path);
> 

your search isn't accurate regarding the error message you got.

the pledge error in dmesg could be read as:
xterm (pid 90461) has been killed due to pledge violation.
the program tried to use syscall 5 using "cpath" promise, but it
didn't pledged for it.

what is the syscall 5 ?

$ grep '5$' /usr/include/sys/syscall.h
#define SYS_open5

So it is an open(2) call which need "cpath".

if it needs "cpath", the flags argument of open(2) should contains
O_CREAT:

kern/vfs_syscalls.c
  1018  if (oflags & O_CREAT)
  1019  ni_pledge |= PLEDGE_CPATH;


so at some point after calling pledge(2), xterm called open(?, ?|O_CREAT).

it could be a direct call to open(2), or an indirect call (like calling
fopen(3) with "w").

if you search in xterm code source, you should find several occurrences
of such calls. But like deraadt@ told you, you should also ensure this
call is effectively *after* the pledge call.

as hint, you know it seems to be triggerable from some keyboard
combinaison. enumerating what xterm does by default for such things
could help too to track the problem.

there is a open(2) call somewhere: the pledge violation is proof of
that. but the solution isn't necessary to allow this file creation (by
bindly extending the pledge promises). it could be to disallow the file
creation.

but to decide, we should know *what* triggered this behaviour.

personally, I like to know that xterm is unable to create a file.

thanks.
-- 
Sebastien Marie



Re: [bugfix] xterm(1) needs "cpath" pledge(2)

2018-07-28 Thread Lauri Tirkkonen
On Sun, Jul 29 2018 07:28:19 +0200, Sebastien Marie wrote:
> personally, I like to know that xterm is unable to create a file.

At least this feature will create files (accessible through the context
menu through ctrl-left click):

Print-All Immediately (resource print-immediate)
   Invokes the print-immediate action, sending the text of
   the current window directly to a file, as specified by
   the printFileImmediate, printModeImmediate and
   printOptsImmediate resources.

and possibly the other "Commands for capturing output" listed in the manpage.

-- 
Lauri Tirkkonen | lotheac @ IRCnet



Re: [bugfix] xterm(1) needs "cpath" pledge(2)

2018-07-28 Thread Matthieu Herrb
On Sun, Jul 29, 2018 at 07:28:19AM +0200, Sebastien Marie wrote:
> Hi,
> 
> First, thanks to (trying) to search for solve the problem you encountered.
> 
> On Sat, Jul 28, 2018 at 10:08:04PM +0300, Leonid Bobrov wrote:
> > Hi!
> > 
> > Like I said yesterday, I don't know how to reproduce this bug, it just
> > happened to me and I got this dmesg(1):
> > xterm[90461]: pledge "cpath", syscall 5
> > 
> > Right now I quickly grep(1)'ed xterm(1)'s source code:
> > mazocomp$ egrep "mkdir\(|unlink\(|rmdir\(" -R . -n
> > ./misc.c:760:   && mkdir(filename, 0700) == 0) {
> > ./misc.c:804:   unlink(xterm_cursor_theme);
> > ./misc.c:805:   rmdir(my_path);
> > 
> 
> your search isn't accurate regarding the error message you got.
> 
> the pledge error in dmesg could be read as:
>   xterm (pid 90461) has been killed due to pledge violation.
>   the program tried to use syscall 5 using "cpath" promise, but it
>   didn't pledged for it.
> 
> what is the syscall 5 ?
> 
> $ grep '5$' /usr/include/sys/syscall.h
> #define SYS_open5
> 
> So it is an open(2) call which need "cpath".
> 
> if it needs "cpath", the flags argument of open(2) should contains
> O_CREAT:
> 
> kern/vfs_syscalls.c
>   1018  if (oflags & O_CREAT)
>   1019  ni_pledge |= PLEDGE_CPATH;
> 
> 
> so at some point after calling pledge(2), xterm called open(?, ?|O_CREAT).
> 
> it could be a direct call to open(2), or an indirect call (like calling
> fopen(3) with "w").
> 
> if you search in xterm code source, you should find several occurrences
> of such calls. But like deraadt@ told you, you should also ensure this
> call is effectively *after* the pledge call.
> 
> as hint, you know it seems to be triggerable from some keyboard
> combinaison. enumerating what xterm does by default for such things
> could help too to track the problem.
> 
> there is a open(2) call somewhere: the pledge violation is proof of
> that. but the solution isn't necessary to allow this file creation (by
> bindly extending the pledge promises). it could be to disallow the file
> creation.
> 
> but to decide, we should know *what* triggered this behaviour.

Hi,

After digging a bit, there is at least the 'Print All Immediatly'
function from button 1 menu that will trigger the creation of a file
and violate  the pledge.

see xtermPrintImmediately() in print.c:789. The fopen() itself appears
in charToPrinter() on line 498 of the same file.

Should this feature be disabled in xterm ?
-- 
Matthieu Herrb



Re: [bugfix] xterm(1) needs "cpath" pledge(2)

2018-07-29 Thread Sebastien Marie
On Sun, Jul 29, 2018 at 08:43:22AM +0200, Matthieu Herrb wrote:
> On Sun, Jul 29, 2018 at 07:28:19AM +0200, Sebastien Marie wrote:
> > 
> > but to decide, we should know *what* triggered this behaviour.
> 
> Hi,
> 
> After digging a bit, there is at least the 'Print All Immediatly'
> function from button 1 menu that will trigger the creation of a file
> and violate  the pledge.
> 
> see xtermPrintImmediately() in print.c:789. The fopen() itself appears
> in charToPrinter() on line 498 of the same file.

Hi Matthieu and Lauri,

I didn't expected someone else did the homework of Leonid so quickly :)
but thanks for digging in this problem.

> Should this feature be disabled in xterm ?

As xterm has an external upstream, just disabling the feature could be
annoying for future merges. And such feature could be legitimate too. I
would like to hear what others think about it.

The problem is this command doesn't have switch we could use to decide
if we add "cpath" in pledge(2) or not ; as we have already done for
exec-formated or exec-selectable commands and "exec" promise.


I wonder if unveil(2) could help here: unveiling for "cpath" only the
directory where file creation could occurs ? But I need to check some
points on unveil(2) first...

Thanks.
-- 
Sebastien Marie



Re: [bugfix] xterm(1) needs "cpath" pledge(2)

2018-07-29 Thread Theo de Raadt
Matthieu Herrb  wrote:

> > but to decide, we should know *what* triggered this behaviour.
> 
> Hi,
> 
> After digging a bit, there is at least the 'Print All Immediatly'
> function from button 1 menu that will trigger the creation of a file
> and violate  the pledge.
> 
> see xtermPrintImmediately() in print.c:789. The fopen() itself appears
> in charToPrinter() on line 498 of the same file.
> 
> Should this feature be disabled in xterm ?

that's probably the best approach.

Our goal is to have the safest xterm possible, rather than an xterm with
features used by less than 0.001% of people, perhaps once every few
months (I am estimating).



Re: [bugfix] xterm(1) needs "cpath" pledge(2)

2018-07-29 Thread Leonid Bobrov
Lauri, Matthieu, thank you for saving me the trouble having to read and
understand the code of this big fat hippopotamus.

So, that means somehow I actually accidentally pressed annoying touchpad
and was lucky enough to click "Print-All Immediately" without noticing
that... I regret that I've sent previous messages (I had a feeling it's
a mistake to send them from the beginning), especially Theo's reaction
made me irritated. I am happy to realise I don't use coloured cursors. I
hope starting thread about this problem didn't hurt anyone besides Theo.

(sarcasm): My mails explicitly show that I am asshole which uses
community as unpaid workforce, today Matthieu and Lauri have made
everything for me.



Re: [bugfix] xterm(1) needs "cpath" pledge(2)

2018-07-29 Thread lists
Sun, 29 Jul 2018 15:46:20 +0300 (EEST) Leonid Bobrov 
> Lauri, Matthieu, thank you for saving me the trouble having to read and
> understand the code of this big fat hippopotamus.
> 
> So, that means somehow I actually accidentally pressed annoying touchpad
> and was lucky enough to click "Print-All Immediately" without noticing
> that... I regret that I've sent previous messages (I had a feeling it's
> a mistake to send them from the beginning), especially Theo's reaction
> made me irritated. I am happy to realise I don't use coloured cursors. I
> hope starting thread about this problem didn't hurt anyone besides Theo.

Hi Leonid,

It's very appropriate for chat sessions obviously debug one this is not.
Hope you can leave your sarcasm out for the next round of ports testing.

Kind regards,
Anton Lazarov

> (sarcasm): My mails explicitly show that I am asshole which uses
> community as unpaid workforce, today Matthieu and Lauri have made
> everything for me.
>