From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp1.rz.uni-karlsruhe.de (Debian-exim@smtp1.rz.uni-karlsruhe.de [129.13.185.217]) by krisdoz.my.domain (8.14.3/8.14.3) with ESMTP id o9PLtBup013850 for ; Mon, 25 Oct 2010 17:55:13 -0400 (EDT) Received: from hekate.usta.de (asta-nat.asta.uni-karlsruhe.de [172.22.63.82]) by smtp1.rz.uni-karlsruhe.de with esmtp (Exim 4.63 #1) id 1PAV0U-0004lk-VN; Mon, 25 Oct 2010 23:55:08 +0200 Received: from donnerwolke.usta.de ([172.24.96.3]) by hekate.usta.de with esmtp (Exim 4.71) (envelope-from ) id 1PAV0U-0003pM-UO for tech@mdocml.bsd.lv; Mon, 25 Oct 2010 23:55:06 +0200 Received: from iris.usta.de ([172.24.96.5] helo=usta.de) by donnerwolke.usta.de with esmtp (Exim 4.69) (envelope-from ) id 1PAV0U-0004li-QY for tech@mdocml.bsd.lv; Mon, 25 Oct 2010 23:55:06 +0200 Received: from schwarze by usta.de with local (Exim 4.71) (envelope-from ) id 1PAV0U-0000x6-Jl for tech@mdocml.bsd.lv; Mon, 25 Oct 2010 23:55:06 +0200 Date: Mon, 25 Oct 2010 23:55:06 +0200 From: Ingo Schwarze To: tech@mdocml.bsd.lv Subject: Re: implement .so Message-ID: <20101025215506.GA12557@iris.usta.de> References: <20101024164057.GF20876@iris.usta.de> <20101024164945.GA25275@britannica.bec.de> <20101024172914.GH20876@iris.usta.de> <20101024173857.GA18657@britannica.bec.de> <20101024180019.GI20876@iris.usta.de> <20101024181502.GA13039@britannica.bec.de> <20101024194129.GJ20876@iris.usta.de> <20101024195135.GA1809@britannica.bec.de> X-Mailinglist: mdocml-tech Reply-To: tech@mdocml.bsd.lv MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20101024195135.GA1809@britannica.bec.de> User-Agent: Mutt/1.5.20 (2009-06-14) Hi Joerg, Joerg Sonnenberger wrote on Sun, Oct 24, 2010 at 09:51:35PM +0200: > As we have seen in Rostock, full .so implementation has some funny side > effects, so I consider it a broken feature. Yes, no doubt. It is broken by design in multiple ways. > I won't oppose a clean patch, we have already seen that it is > straight forward to implement. See below, feedback is welcome. After the first round of review and polishing here i'm planning to commit it to OpenBSD and have it looked over by a few people round here. > My point is that it essentially falls apart if you want to use > compressed man pages. Sure, i understand that. > So it really, really shouldn't be used. I fully agree. This is to deal with existing stuff and should not be used for new projects. > Sorry, yes. Only allow relative path names without /../ components. Actually, it is even worse. All ports in the OpenBSD ports tree using the feature give the path relative to the *parent* directory of the directory containing the manual page having the .so directive. So these are neither absolute paths (relative to the root of the file system) nor relative paths (relative to the current working directory). What i'm doing for now is: * regarding validation of the .so directive (roff.c, roff_so()): - path must start with "man" - path must contain exactly one '/' - the characters before and after the slash must not be '.' - the slash must not be the last character * regarding validation of the path to the page containing the .so directive (main.c, pfile()): - path must contain at least one '/' - component before the slash must start with "man" In this case, i replace the last two components of the path to the page containing the .so directive by the contents of the directive, or else error out. Note that the patch builds upon the preceding refactoring patch i sent yesterday, both building on the OpenBSD tree. Yours, Ingo diff -Napur mandoc.pdesc/main.c mandoc/main.c --- mandoc.pdesc/main.c Sun Oct 24 13:47:06 2010 +++ mandoc/main.c Mon Oct 25 15:02:52 2010 @@ -15,6 +15,7 @@ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#include #include #include #include @@ -173,6 +174,8 @@ static const char * const mandocerrs[MANDOCERR_MAX] = "argument count wrong, violates syntax", "child violates parent syntax", "argument count wrong, violates syntax", + "invalid path in include directive", + "include used by a file at an invalid path", "no document body", "no document prologue", "static buffer exhausted", @@ -181,6 +184,7 @@ static const char * const mandocerrs[MANDOCERR_MAX] = static void pdesc(struct curparse *); static void fdesc(struct curparse *); static void ffile(const char *, struct curparse *); +static int pfile(const char *, struct curparse *, int); static int moptions(enum intt *, char *); static int mmsg(enum mandocerr, void *, int, int, const char *); @@ -306,7 +310,58 @@ ffile(const char *file, struct curparse *curp) perror(curp->file); } +static int +pfile(const char *file, struct curparse *curp, int ln) +{ + char path[MAXPATHLEN]; + const char *savefile; + char *p; + int savefd; + if (strlcpy(path, curp->file, sizeof(path)) >= sizeof(path)) { + mmsg(MANDOCERR_MEM, curp, ln, 1, NULL); + return(0); + } + + if (NULL == (p = strrchr(path, '/'))) { + mmsg(MANDOCERR_SOUSER, curp, ln, 1, NULL); + return(0); + } + + while (path < p && '/' != p[-1]) + p--; + + if (strncmp(p, "man", 3)) { + mmsg(MANDOCERR_SOUSER, curp, ln, 1, NULL); + return(0); + } + *p = '\0'; + + if (strlcat(path, file, sizeof(path)) >= sizeof(path)) { + mmsg(MANDOCERR_MEM, curp, ln, 1, NULL); + return(0); + } + + savefile = curp->file; + savefd = curp->fd; + + curp->file = path; + if (-1 == (curp->fd = open(curp->file, O_RDONLY, 0))) { + perror(curp->file); + exit_status = MANDOCLEVEL_SYSERR; + } else { + pdesc(curp); + if (-1 == close(curp->fd)) + perror(curp->file); + } + + curp->file = savefile; + curp->fd = savefd; + + return(MANDOCLEVEL_FATAL > exit_status ? 1 : 0); +} + + static void resize_buf(struct buf *buf, size_t initial) { @@ -629,6 +684,11 @@ pdesc(struct curparse *curp) } else if (ROFF_ERR == re) { assert(MANDOCLEVEL_FATAL <= exit_status); break; + } else if (ROFF_SO == re) { + if (pfile(ln.buf + of, curp, lnn_start)) + continue; + else + break; } /* diff -Napur mandoc.pdesc/mandoc.h mandoc/mandoc.h --- mandoc.pdesc/mandoc.h Sun Oct 24 13:47:06 2010 +++ mandoc/mandoc.h Mon Oct 25 14:57:22 2010 @@ -114,6 +114,8 @@ enum mandocerr { MANDOCERR_SYNTARGVCOUNT, /* argument count wrong, violates syntax */ MANDOCERR_SYNTCHILD, /* child violates parent syntax */ MANDOCERR_SYNTARGCOUNT, /* argument count wrong, violates syntax */ + MANDOCERR_SOPATH, /* invalid path in include directive */ + MANDOCERR_SOUSER, /* include used by a file at an invalid path */ MANDOCERR_NODOCBODY, /* no document body */ MANDOCERR_NODOCPROLOG, /* no document prologue */ MANDOCERR_MEM, /* static buffer exhausted */ diff -Napur mandoc.pdesc/roff.c mandoc/roff.c --- mandoc.pdesc/roff.c Wed Oct 20 09:22:11 2010 +++ mandoc/roff.c Mon Oct 25 14:38:24 2010 @@ -48,11 +48,12 @@ enum rofft { ROFF_ie, ROFF_if, ROFF_ig, + ROFF_nr, ROFF_rm, + ROFF_so, ROFF_tr, ROFF_cblock, ROFF_ccond, /* FIXME: remove this. */ - ROFF_nr, ROFF_MAX }; @@ -128,6 +129,7 @@ static int roff_res(struct roff *, char **, size_t *, int); static void roff_setstr(struct roff *, const char *, const char *); +static enum rofferr roff_so(ROFF_ARGS); static char *roff_strdup(const char *); /* See roff_hash_find() */ @@ -150,11 +152,12 @@ static struct roffmac roffs[ROFF_MAX] = { { "ie", roff_cond, roff_cond_text, roff_cond_sub, ROFFMAC_STRUCT, NULL }, { "if", roff_cond, roff_cond_text, roff_cond_sub, ROFFMAC_STRUCT, NULL }, { "ig", roff_block, roff_block_text, roff_block_sub, 0, NULL }, + { "nr", roff_nr, NULL, NULL, 0, NULL }, { "rm", roff_line, NULL, NULL, 0, NULL }, + { "so", roff_so, NULL, NULL, 0, NULL }, { "tr", roff_line, NULL, NULL, 0, NULL }, { ".", roff_cblock, NULL, NULL, 0, NULL }, { "\\}", roff_ccond, NULL, NULL, 0, NULL }, - { "nr", roff_nr, NULL, NULL, 0, NULL }, }; static void roff_free1(struct roff *); @@ -1005,6 +1008,26 @@ roff_nr(ROFF_ARGS) } return(ROFF_IGN); +} + + +/* ARGSUSED */ +static enum rofferr +roff_so(ROFF_ARGS) +{ + char *name, *p; + + name = *bufp + pos; + if (strncmp(name, "man", 3) || + NULL == (p = strchr(name, '/')) || + '.' == p[-1] || '.' == p[1] || '\0' == p[1] || + strchr(p+1, '/')) { + (*r->msg)(MANDOCERR_SOPATH, r->data, ln, pos, NULL); + return(ROFF_ERR); + } + + *offs = pos; + return(ROFF_SO); } diff -Napur mandoc.pdesc/roff.h mandoc/roff.h --- mandoc.pdesc/roff.h Wed Oct 20 09:22:11 2010 +++ mandoc/roff.h Mon Oct 25 03:48:01 2010 @@ -20,6 +20,7 @@ enum rofferr { ROFF_CONT, /* continue processing line */ ROFF_RERUN, /* re-run roff interpreter with offset */ + ROFF_SO, /* include another file */ ROFF_IGN, /* ignore current line */ ROFF_ERR /* badness: puke and stop */ }; -- To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv