man.cgi: clean exit when absent or empty manpath.conf

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

man.cgi: clean exit when absent or empty manpath.conf

Sebastien Marie
Hi,

Starting to play with man.cgi (src/usr.bin/mandoc/cgi.c), it seems that
man.cgi will segfault if configuration file is absent or empty.

Here a patch that display error message and 505, like when MAN_DIR is
invalid.

Note: the segfault occurs in cgi.c:224 (http_parse), that assume
req->p is not NULL (req->q.manpath = req->p[0]).

The diff use the same style that when MAN_DIR is invalid (cgi.c:917), but that could
be improved using err(3) ?

Thanks.
--
S├ębastien Marie


Index: cgi.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/cgi.c,v
retrieving revision 1.13
diff -u -p -r1.13 cgi.c
--- cgi.c 13 Jul 2014 15:38:06 -0000 1.13
+++ cgi.c 18 Jul 2014 06:43:14 -0000
@@ -966,8 +968,12 @@ pathgen(struct req *req)
  char *dp;
  size_t dpsz;
 
- if (NULL == (fp = fopen("manpath.conf", "r")))
- return;
+ if (NULL == (fp = fopen("manpath.conf", "r"))) {
+ fprintf(stderr, "manpath.conf not found in MAN_DIR (%s)\n",
+                    MAN_DIR);
+ pg_error_internal();
+ exit(EXIT_FAILURE);
+ }
 
  while (NULL != (dp = fgetln(fp, &dpsz))) {
  if ('\n' == dp[dpsz - 1])
@@ -975,5 +981,11 @@ pathgen(struct req *req)
  req->p = mandoc_realloc(req->p,
     (req->psz + 1) * sizeof(char *));
  req->p[req->psz++] = mandoc_strndup(dp, dpsz);
+ }
+
+ if ( req->p == NULL ) {
+ fprintf(stderr, "empty manpath.conf\n");
+ pg_error_internal();
+ exit(EXIT_FAILURE);
  }
 }

Reply | Threaded
Open this post in threaded view
|

Re: man.cgi: clean exit when absent or empty manpath.conf

Ingo Schwarze
Salut Sebastien,

S├ębastien Marie wrote on Fri, Jul 18, 2014 at 08:50:17AM +0200:

> Starting to play with man.cgi (src/usr.bin/mandoc/cgi.c),

Thank you for testing.

> it seems that man.cgi will segfault if configuration file
> is absent or empty.
>
> Here a patch that display error message and 505, like when MAN_DIR is
> invalid.
>
> Note: the segfault occurs in cgi.c:224 (http_parse), that assume
> req->p is not NULL (req->q.manpath = req->p[0]).
>
> The diff use the same style that when MAN_DIR is invalid (cgi.c:917),

That is all very much to the point, thank you for finding and
analysing the bug and providing a correct patch.

I have tweaked the message strings a bit, committed your patch
to both cvs.openbsd.org and mdocml.bsd.lv, and updated the installed
man.cgi programs on both www.openbsd.org and mdocml.bsd.lv.

> but that could be improved using err(3) ?

Well, i do like the err(3) family of functions for its clarity
and conciseness, but so far, we have refrained from using them
in mandoc for portability reasons.  While it is trivial to
provide compat functions for systems not having them, it is
just as trivial to use fprintf(3), strerror(3), and exit(3)
for now, so this is kind of a bikeshed.  I guess at some point
i will switch to using err(3), even POSIX considers adding them
unless i heard an unfounded rumour...

Yours,
  Ingo