On Wed, Oct 24, 2018 at 11:24:59AM +0100, Ricardo Mestre wrote: > Hi, > > If input_name is provided we can unveil it with read permissions, if > output_name is provided we need to unveil this one with rwc. Additionally > depending on the different combinations of if these files are passed via args > or from stdin/to stdout we can also pledge accordingly to the code path. This > has been tested succefully with bdf fonts we have bundled in > xenocara.
This one looks ok, but it lacks autoconf support to allow the
application to build on other systems.
>
> Since I have several other X apps unveiled and/or pledged could you please
> comment not only with the unveil/pledge part, but also err vs fprintf/exit,
> the
> placement of the #includes and also tabs vs spaces?
Generally, I'm not too found of pledging/unveiling random X client
programs. There are a lot of "hidden" features in X libraries that
will probably break with too strict pledges and/or unveils.
Also since this is OpenBSD-specific, it will be difficult to get it
upstreams, especially if you don't provide the autoconf goo to make
the code still build/work on Linux. And when not upstreaming it
creates more burden to merge new versions of the applications.
>
> Index: bdftopcf.c
> ===================================================================
> RCS file: /cvs/xenocara/app/bdftopcf/bdftopcf.c,v
> retrieving revision 1.5
> diff -u -p -u -r1.5 bdftopcf.c
> --- bdftopcf.c 29 Mar 2018 20:34:30 -0000 1.5
> +++ bdftopcf.c 24 Oct 2018 10:15:41 -0000
> @@ -38,7 +38,9 @@ from The Open Group.
> #include "fntfil.h"
> #include "bdfint.h"
> #include "pcf.h"
> +#include <err.h>
> #include <stdio.h>
> +#include <unistd.h>
> #include <X11/Xos.h>
>
> int
> @@ -158,6 +160,26 @@ main(int argc, char *argv[])
> }
> argv++;
> }
> +
> + if (input_name) {
> + if (unveil(input_name, "r") == -1)
> + err(1, "unveil");
> + }
> + if (output_name) {
> + if (unveil(output_name, "rwc") == -1)
> + err(1, "unveil");
> + if (pledge("stdio rpath wpath cpath", NULL) == -1)
> + err(1, "pledge");
> + }
> + if (input_name && !output_name) {
> + if (pledge("stdio rpath", NULL) == -1)
> + err(1, "pledge");
> + }
> + if (!input_name && !output_name) {
> + if (pledge("stdio", NULL) == -1)
> + err(1, "pledge");
> + }
> +
> if (input_name) {
> input = FontFileOpen(input_name);
> if (!input) {
--
Matthieu Herrb
signature.asc
Description: PGP signature
