From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from krisdoz.my.domain (schwarze@localhost [127.0.0.1]) by krisdoz.my.domain (8.14.5/8.14.5) with ESMTP id s6MIEDoZ029169 for ; Tue, 22 Jul 2014 14:14:13 -0400 (EDT) Received: (from schwarze@localhost) by krisdoz.my.domain (8.14.5/8.14.3/Submit) id s6MIEDOk016020; Tue, 22 Jul 2014 14:14:13 -0400 (EDT) Date: Tue, 22 Jul 2014 14:14:13 -0400 (EDT) Message-Id: <201407221814.s6MIEDOk016020@krisdoz.my.domain> X-Mailinglist: mdocml-source Reply-To: source@mdocml.bsd.lv MIME-Version: 1.0 From: schwarze@mdocml.bsd.lv To: source@mdocml.bsd.lv Subject: mdocml: Security fix to prevent XSS attacks: Restrict the character set X-Mailer: activitymail 1.26, http://search.cpan.org/dist/activitymail/ Content-Type: text/plain; charset=utf-8 Log Message: ----------- Security fix to prevent XSS attacks: Restrict the character set of strings passed into html_alloc(), in particular architecture names that come from the QUERY_STRING, but also SCRIPT_NAME and manpath.conf content for additional safety, and bail out safely on violations. Issue reported by Sebastien Marie . Modified Files: -------------- mdocml: cgi.c man.cgi.8 Revision Data ------------- Index: man.cgi.8 =================================================================== RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/man.cgi.8,v retrieving revision 1.8 retrieving revision 1.9 diff -Lman.cgi.8 -Lman.cgi.8 -u -p -r1.8 -r1.9 --- man.cgi.8 +++ man.cgi.8 @@ -267,6 +267,34 @@ For backward compatibility with the trad is supported as an alias for .Cm sec . .El +.Ss Restricted character set +For security reasons, in particular to prevent cross site scripting +attacks, some strings used by +.Nm +can only contain the following characters: +.Pp +.Bl -dash -compact -offset indent +.It +lower case and upper case ASCII letters +.It +the ten decimal digits +.It +the dash +.Pq Sq - +.It +the dot +.Pq Sq \&. +.It +the slash +.Pq Sq / +.It +the underscore +.Pq Sq _ +.El +.Pp +In particular, this applies to the +.Ev SCRIPT_NAME , +to all manpaths, and to all architecture names. .Sh ENVIRONMENT The web server may pass the following CGI variables to .Nm : @@ -293,6 +321,10 @@ binary relative to the server root, usua .Pa /cgi-bin/man.cgi . This is used for generating URIs to be embedded in generated HTML code and HTTP headers. +If this contains any character not contained in the +.Sx Restricted character set , +.Nm +reports an internal server error and exits without doing anything. .El .Sh FILES .Bl -tag -width Ds @@ -332,6 +364,12 @@ Manual pages documenting itself, linked from the index page. .It Pa /man/manpath.conf The list of available manpaths, one per line. +If any of the lines in this file contains a slash +.Pq Sq / +or any character not contained in the +.Sx Restricted character set , +.Nm +reports an internal server error and exits without doing anything. .It Pa /man/OpenBSD-current/man1/mandoc.1 An example .Xr mdoc 7 Index: cgi.c =================================================================== RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/cgi.c,v retrieving revision 1.79 retrieving revision 1.80 diff -Lcgi.c -Lcgi.c -u -p -r1.79 -r1.80 --- cgi.c +++ cgi.c @@ -467,6 +467,20 @@ resp_searchform(const struct req *req) } static int +validate_urifrag(const char *frag) +{ + + while ('\0' != *frag) { + if ( ! (isalnum((unsigned char)*frag) || + '-' == *frag || '.' == *frag || + '/' == *frag || '_' == *frag)) + return(0); + frag++; + } + return(1); +} + +static int validate_manpath(const struct req *req, const char* manpath) { size_t i; @@ -960,6 +974,13 @@ main(void) if (NULL == (scriptname = getenv("SCRIPT_NAME"))) scriptname = ""; + if ( ! validate_urifrag(scriptname)) { + fprintf(stderr, "unsafe SCRIPT_NAME \"%s\"\n", + scriptname); + pg_error_internal(); + return(EXIT_FAILURE); + } + /* * First we change directory into the MAN_DIR so that * subsequent scanning for manpath directories is rooted @@ -987,6 +1008,12 @@ main(void) return(EXIT_FAILURE); } + if ( ! (NULL == req.q.arch || validate_urifrag(req.q.arch))) { + pg_error_badrequest( + "You specified an invalid architecture."); + return(EXIT_FAILURE); + } + /* Dispatch to the three different pages. */ path = getenv("PATH_INFO"); @@ -1038,7 +1065,20 @@ pathgen(struct req *req) dpsz--; req->p = mandoc_realloc(req->p, (req->psz + 1) * sizeof(char *)); - req->p[req->psz++] = mandoc_strndup(dp, dpsz); + dp = mandoc_strndup(dp, dpsz); + if ( ! validate_urifrag(dp)) { + fprintf(stderr, "%s/manpath.conf contains " + "unsafe path \"%s\"\n", MAN_DIR, dp); + pg_error_internal(); + exit(EXIT_FAILURE); + } + if (NULL != strchr(dp, '/')) { + fprintf(stderr, "%s/manpath.conf contains " + "path with slash \"%s\"\n", MAN_DIR, dp); + pg_error_internal(); + exit(EXIT_FAILURE); + } + req->p[req->psz++] = dp; } if ( req->p == NULL ) { -- To unsubscribe send an email to source+unsubscribe@mdocml.bsd.lv