Author: rmeggins Update of /cvs/dirsec/dsgw In directory cvs1.fedora.phx.redhat.com:/tmp/cvs-serv11172/dsgw Modified Files: cgiutil.c cookie.c csearch.c dnedit.c doauth.c domodify.c dosearch.c dsgw.h dsgwutil.c emitf.c entrydisplay.c htmlout.c ldaputil.c search.c tutor.c Log Message: Resolves: bug 472092 Bug Description: DSGW password corruption Reviewed by: nkinder (Thanks!) Fix Description: 1) By default, all of the get/post parameters have the html entities escaped, so we can be sure that they are displayed to the user escaped, to avoid XSS issues. However, values sent to LDAP must be unescaped. The doauth code is used to authenticate directory manager and ordinary users, so we have to unescape the password explicitly there. The domodify code is used when data is added or modified in the directory server. It's easier to just fix all of the values before sending to the directory server. 2) The entity code has been moved to adminutil, so use the adminutil functions instead of the dsgw functions. This will require adminutil 1.1.8. 3) Clean up various compiler warnings. Platforms tested: RHEL5 Flag Day: no Doc impact: no Index: cgiutil.c =================================================================== RCS file: /cvs/dirsec/dsgw/cgiutil.c,v retrieving revision 1.5 retrieving revision 1.6 diff -u -r1.5 -r1.6 --- cgiutil.c 27 Feb 2008 03:36:50 -0000 1.5 +++ cgiutil.c 22 Dec 2008 21:44:30 -0000 1.6 @@ -152,7 +152,7 @@ } #ifdef DSGW_DEBUG - dsgw_log ("vars=\"%s\"\n", vars); + dsgw_log ("vars=\"%p\"\n", vars); #endif vars = get_input_ptr(); dsgw_vec_convert (vars); /* convert to utf8 */ Index: cookie.c =================================================================== RCS file: /cvs/dirsec/dsgw/cookie.c,v retrieving revision 1.1.1.1 retrieving revision 1.2 diff -u -r1.1.1.1 -r1.2 --- cookie.c 1 Jun 2006 19:43:39 -0000 1.1.1.1 +++ cookie.c 22 Dec 2008 21:44:30 -0000 1.2 @@ -301,7 +301,7 @@ expired = 1; } else { #ifdef DSGW_DEBUG - dsgw_log( "dsgw_ckdn2passwd: cookie expired (%ld > %ld) but within domodify grace period\n", now, atoi( lifetimestr )); + dsgw_log( "dsgw_ckdn2passwd: cookie expired (%ld > %ld) but within domodify grace period\n", now, atol( lifetimestr )); #endif } } else if ( now > atoi( lifetimestr )) { @@ -311,7 +311,7 @@ if ( expired != 0 ) { dsgw_closecookiedb( fp ); #ifdef DSGW_DEBUG - dsgw_log( "dsgw_ckdn2passwd: expired (%ld > %ld)\n", now, atoi( lifetimestr )); + dsgw_log( "dsgw_ckdn2passwd: expired (%ld > %ld)\n", now, atol( lifetimestr )); #endif return DSGW_CKDB_EXPIRED; } Index: csearch.c =================================================================== RCS file: /cvs/dirsec/dsgw/csearch.c,v retrieving revision 1.7 retrieving revision 1.8 diff -u -r1.7 -r1.8 --- csearch.c 20 Mar 2008 02:18:39 -0000 1.7 +++ csearch.c 22 Dec 2008 21:44:30 -0000 1.8 @@ -41,6 +41,9 @@ #include "dsgw.h" #include "dbtdsgw.h" +#ifdef DSGW_DEBUG +#include <unistd.h> +#endif static void get_request(char *fname); static void emit_file(char* filename, struct ldap_searchobj* sop); Index: dnedit.c =================================================================== RCS file: /cvs/dirsec/dsgw/dnedit.c,v retrieving revision 1.7 retrieving revision 1.8 diff -u -r1.7 -r1.8 --- dnedit.c 20 Mar 2008 02:18:39 -0000 1.7 +++ dnedit.c 22 Dec 2008 21:44:30 -0000 1.8 @@ -120,8 +120,7 @@ * * I feel your pain, so I have removed the pain. */ - "var comp_js = 'CU'\n", - dsgw_getvp( DSGW_CGINUM_EDIT ), context, edn ); + "var comp_js = 'CU'\n" ); dsgw_emits("var dnlist = new Array;\n" ); for ( i = 0; attrvals && attrvals[ i ] != NULL; i++ ) { xdn = ldap_explode_dn( attrvals[ i ], 1 ); Index: doauth.c =================================================================== RCS file: /cvs/dirsec/dsgw/doauth.c,v retrieving revision 1.6 retrieving revision 1.7 diff -u -r1.6 -r1.7 --- doauth.c 12 Jun 2008 14:01:33 -0000 1.6 +++ doauth.c 22 Dec 2008 21:44:30 -0000 1.7 @@ -76,6 +76,9 @@ encodeddn = dsgw_strdup_escaped( binddn ); authdesturl = dsgw_get_cgi_var( "authdesturl", DSGW_CGIVAR_OPTIONAL ); password = dsgw_get_cgi_var( "password", DSGW_CGIVAR_OPTIONAL ); + if (password && password[0]) { + unescape_entities(password); + } (void) dsgw_init_ldap( &ld, NULL, 1, 0); Index: domodify.c =================================================================== RCS file: /cvs/dirsec/dsgw/domodify.c,v retrieving revision 1.4 retrieving revision 1.5 diff -u -r1.4 -r1.5 --- domodify.c 6 Mar 2008 22:00:09 -0000 1.4 +++ domodify.c 22 Dec 2008 21:44:30 -0000 1.5 @@ -93,13 +93,13 @@ post_request() { LDAP *ld; - int rc, changetype, dnlen, i, passwd_changed, discard_authcreds; + int rc, changetype, dnlen, i, passwd_changed; char *s, *encodeddn, *dn, *newrdn, *changedesc, **rdns, **oldrdns, *jscomp, *entry_name, *new_name, *success_msg; char *old_dn; char buf[ 256 ]; - passwd_changed = discard_authcreds = 0; + passwd_changed = 0; s = dsgw_get_cgi_var( "changetype", DSGW_CGIVAR_REQUIRED ); changedesc = XP_GetClientStr(DBT_Editing_); @@ -553,16 +553,18 @@ return(LDAP_PARAM_ERROR); } - if ( verbose && pmods != NULL ) { + if ( pmods != NULL ) { int j, notascii; unsigned long k; struct berval *bvp; for ( i = 0; pmods[ i ] != NULL; ++i ) { modop = pmods[ i ]->mod_op & ~LDAP_MOD_BVALUES; - dsgw_emitf( "%s %s:\n", modop == LDAP_MOD_REPLACE ? - "replace" : modop == LDAP_MOD_ADD ? - "add" : "delete", pmods[ i ]->mod_type ); + if (verbose) { + dsgw_emitf( "%s %s:\n", modop == LDAP_MOD_REPLACE ? + "replace" : modop == LDAP_MOD_ADD ? + "add" : "delete", pmods[ i ]->mod_type ); + } if ( pmods[ i ]->mod_bvalues != NULL ) { for ( j = 0; pmods[ i ]->mod_bvalues[ j ] != NULL; ++j ) { bvp = pmods[ i ]->mod_bvalues[ j ]; @@ -573,10 +575,16 @@ break; } } - if ( notascii ) { - dsgw_emitf( XP_GetClientStr(DBT_TnotAsciiLdBytesN_), bvp->bv_len ); - } else { - dsgw_emitf( "\t\"%s\"\n", bvp->bv_val ); + if (verbose) { + if ( notascii ) { + dsgw_emitf( XP_GetClientStr(DBT_TnotAsciiLdBytesN_), bvp->bv_len ); + } else { + dsgw_emitf( "\t\"%s\"\n", bvp->bv_val ); + } + } + /* make sure all values sent via LDAP are not html escaped */ + if (!notascii && bvp->bv_val) { /* not not ascii == ascii */ + unescape_entities(bvp->bv_val); } } } Index: dosearch.c =================================================================== RCS file: /cvs/dirsec/dsgw/dosearch.c,v retrieving revision 1.5 retrieving revision 1.6 diff -u -r1.5 -r1.6 --- dosearch.c 27 Feb 2008 03:36:50 -0000 1.5 +++ dosearch.c 22 Dec 2008 21:44:30 -0000 1.6 @@ -139,9 +139,6 @@ static void get_request(char *dn, char *ldapquery) { - int urllen = 0; - int argslen = 0; - char *p = NULL; char *ldapurl = NULL; /* Index: dsgw.h =================================================================== RCS file: /cvs/dirsec/dsgw/dsgw.h,v retrieving revision 1.10 retrieving revision 1.11 diff -u -r1.10 -r1.11 --- dsgw.h 4 Sep 2008 02:46:09 -0000 1.10 +++ dsgw.h 22 Dec 2008 21:44:30 -0000 1.11 @@ -874,8 +874,6 @@ #else ; #endif -char *dsgw_strdup_with_entities( char *s, int *madecopyp ); -void dsgw_convert_entities( char *s ); void dsgw_HTML_emits( char * ); void dsgw_emit_cgi_var( int argc, char **argv ); void dsgw_emit_button( int argc, char **argv, const char* format, ... ) Index: dsgwutil.c =================================================================== RCS file: /cvs/dirsec/dsgw/dsgwutil.c,v retrieving revision 1.13 retrieving revision 1.14 diff -u -r1.13 -r1.14 --- dsgwutil.c 4 Sep 2008 02:46:09 -0000 1.13 +++ dsgwutil.c 22 Dec 2008 21:44:30 -0000 1.14 @@ -40,6 +40,7 @@ * dsgwutil.c -- misc. utility functions -- HTTP gateway */ +#include <unistd.h> #include <limits.h> /* PATH_MAX */ #include "dsgw.h" #include "dbtdsgw.h" Index: emitf.c =================================================================== RCS file: /cvs/dirsec/dsgw/emitf.c,v retrieving revision 1.2 retrieving revision 1.3 diff -u -r1.2 -r1.3 --- emitf.c 14 Jan 2008 22:58:30 -0000 1.2 +++ emitf.c 22 Dec 2008 21:44:30 -0000 1.3 @@ -741,7 +741,8 @@ if (*s == 'q' || *s == 'Q') { while (ldap_utf8isspace (LDAP_UTF8INC(s))); if (*s == '=') { - item[i].i_q = strtod(++s, &s); + ++s; + item[i].i_q = strtod(s, &s); } } } while ((s = strchr (s, ';')) != NULL); Index: entrydisplay.c =================================================================== RCS file: /cvs/dirsec/dsgw/entrydisplay.c,v retrieving revision 1.11 retrieving revision 1.12 diff -u -r1.11 -r1.12 --- entrydisplay.c 22 Dec 2008 19:50:15 -0000 1.11 +++ entrydisplay.c 22 Dec 2008 21:44:30 -0000 1.12 @@ -739,7 +739,7 @@ if ( !editable ) { char *urlprefix = dsgw_ch_malloc( strlen(gc->gc_urlpfxmain) + 128); sprintf(urlprefix, "%semptyFrame.html", gc->gc_urlpfxmain); - dsgw_convert_entities(urlprefix); + unescape_entities(urlprefix); /* include the functions used to support "Edit" buttons */ /* function haveAuthCookie() */ @@ -1537,7 +1537,7 @@ int freeit; if ( quote_html_specials ) { - val = dsgw_strdup_with_entities( val, &freeit ); + val = strdup_escape_entities( val, &freeit ); } else { freeit = 0; } Index: htmlout.c =================================================================== RCS file: /cvs/dirsec/dsgw/htmlout.c,v retrieving revision 1.7 retrieving revision 1.8 diff -u -r1.7 -r1.8 --- htmlout.c 11 Jun 2008 21:12:31 -0000 1.7 +++ htmlout.c 22 Dec 2008 21:44:30 -0000 1.8 @@ -176,7 +176,7 @@ } } - newlabel = dsgw_strdup_with_entities( label, &freenewlabel ); + newlabel = strdup_escape_entities( label, &freenewlabel ); if ( newlabel != NULL && *newlabel != '\0' ) { dsgw_emitf( ">%s</A>\n", newlabel ); if ( freenewlabel ) { @@ -266,98 +266,6 @@ } -#define DSGW_MAX_ENTITY_LEN 6 /* &quot; */ -static char *specials = "&\"<>"; -static char *entities[] = { "&amp;", "&quot;", "&lt;", "&gt;" }; -static int entitylen[] = { 5, 6, 4, 4 }; -static int entitynum = sizeof(entities)/sizeof(entities[0]); - -char * -dsgw_strdup_with_entities( char *s, int *madecopyp ) -{ -/* - * If the UTF8 string "s" contains any HTML special characters, make a - * duplicate where the appropriate HTML "entities" have been substituted - * for the special chars. For example, &quot;&lt;mcs(a)ace.com&gt;&quot; will be translated - * to &quot;&amp;lt;mcs(a)ace.com&amp;gt;&quot;. - * - * If "s" does not contain any special characters, it is returned and - * *madecopyp is set to 0. - * Otherwise a malloc'd string is returned and *madecopyp is set to 1. - */ - int spcount, idx; - char *p, *q, *r, *d; - - spcount = 0; - for ( p = s; *p != '\0'; LDAP_UTF8INC( p )) { - if ( ((*p) & 0x80) == 0 && strchr( specials, *p ) != NULL ) { - ++spcount; - } - } - - if ( spcount == 0 ) { - *madecopyp = 0; - return( s ); - } - - d = r = dsgw_ch_malloc( strlen( s ) + 1 + spcount * DSGW_MAX_ENTITY_LEN ); - for ( p = s; *p != '\0'; LDAP_UTF8INC( p )) { - if ( ((*p) & 0x80) == 0 && ( q = strchr( specials, *p )) != NULL ) { - idx = ( q - specials ); - memcpy( r, entities[ idx ], entitylen[ idx ] ); - r += entitylen[ idx ]; - } else { - r += LDAP_UTF8COPY( r, p ); - } - } - *r = '\0'; - - *madecopyp = 1; - return( d ); -} - -/* this will convert a string with escaped entities ("&amp;") - back to the original unescaped string ("&") - This is necessary for converting URLs which would normally - have entities in them (e.g. search?context=foo&amp;dn=bar) - for use in javascript (e.g. window.href = 'search?context=foo&dn=bar') - since javascript must use the unescaped version - This converts the string in place since the entities "&amp;" - take up much more room than the single character represented - If you need to work on a copy then make a copy with strdup first. -*/ -void -dsgw_convert_entities(char *s) -{ - int spcount, idx; - char *p, *q, *r, *d; - - if (!s || !*s) { - return; - } - - d = r = s; - for ( p = s; *p != '\0'; LDAP_UTF8INC( p )) { - if ( ((*p) & 0x80) == 0 && ( (*p) == '&') ) { - for( idx = 0; idx < entitynum; ++idx ) { - if (!strncmp(p, entities[ idx ], entitylen[ idx ])) { - break; - } - } - if (idx < entitynum) { - *r = specials[idx]; - ++r; - p += entitylen[ idx ]-1; /* the 1 will be added in the for loop */ - } else { - r += LDAP_UTF8COPY( r, p ); - } - } else { - r += LDAP_UTF8COPY( r, p ); - } - } - *r = '\0'; -} - void dsgw_form_begin( const char* name, const char* format, ... ) { Index: ldaputil.c =================================================================== RCS file: /cvs/dirsec/dsgw/ldaputil.c,v retrieving revision 1.5 retrieving revision 1.6 diff -u -r1.5 -r1.6 --- ldaputil.c 6 Mar 2008 22:00:09 -0000 1.5 +++ ldaputil.c 22 Dec 2008 21:44:30 -0000 1.6 @@ -155,6 +155,9 @@ } rndstr = dn = NULL; passwd = dsgw_get_cgi_var( "passwd", DSGW_CGIVAR_OPTIONAL ); + if (passwd && passwd[0]) { + unescape_entities(passwd); /* unescape before using with ldap */ + } if (( p = dsgw_get_cgi_var( "ldapsizelimit", DSGW_CGIVAR_OPTIONAL )) != NULL ) { @@ -189,7 +192,7 @@ #ifdef DSGW_DEBUG dsgw_log( "dsgw_init_ldap: run under admserv, user id = %s, " - "dn = %s, passwd = %s, skipac = %d, dn = 0x%x\n", + "dn = %s, passwd = %s, skipac = %d, dn = 0x%p\n", userid == NULL ? "NULL" : userid, dn == NULL ? "NULL" : dn, passwd == NULL ? "NULL" : passwd, Index: search.c =================================================================== RCS file: /cvs/dirsec/dsgw/search.c,v retrieving revision 1.4 retrieving revision 1.5 diff -u -r1.4 -r1.5 --- search.c 6 Mar 2008 22:00:09 -0000 1.4 +++ search.c 22 Dec 2008 21:44:30 -0000 1.5 @@ -38,9 +38,11 @@ /* * search.c -- CGI program to generate smart search form -- HTTP gateway */ - #include "dsgw.h" #include "dbtdsgw.h" +#ifdef DSGW_DEBUG +#include <unistd.h> +#endif static void get_request(char *docname); static void do_searchtype_popup( struct ldap_searchobj *sop ); Index: tutor.c =================================================================== RCS file: /cvs/dirsec/dsgw/tutor.c,v retrieving revision 1.5 retrieving revision 1.6 diff -u -r1.5 -r1.6 --- tutor.c 20 Mar 2008 02:18:39 -0000 1.5 +++ tutor.c 22 Dec 2008 21:44:30 -0000 1.6 @@ -198,7 +198,7 @@ * Where MANUAL is literal */ html = PL_strdup(gc->gc_urlpfxmain); - dsgw_convert_entities(html); + unescape_entities(html); dsgw_emitf("Location: %s%s/%s\n\n", html, DSGW_MANUALSHORTCUT, head); free(html);