From b1d087c3ea0c9bd3cd5563f0d519b93cdf50882b Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Dec 22 2008 21:44:30 +0000 Subject: 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 --- diff --git a/cgiutil.c b/cgiutil.c index 2652f4a..99af2c7 100644 --- a/cgiutil.c +++ b/cgiutil.c @@ -152,7 +152,7 @@ dsgw_post_begin(FILE *in) } #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 */ diff --git a/cookie.c b/cookie.c index da7d2a8..0e850cb 100644 --- a/cookie.c +++ b/cookie.c @@ -301,7 +301,7 @@ dsgw_ckdn2passwd( char *rndstr, char *dn, char **ret_pw ) 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 @@ dsgw_ckdn2passwd( char *rndstr, char *dn, char **ret_pw ) 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; } diff --git a/csearch.c b/csearch.c index 5905d0f..cc9b922 100644 --- a/csearch.c +++ b/csearch.c @@ -41,6 +41,9 @@ #include "dsgw.h" #include "dbtdsgw.h" +#ifdef DSGW_DEBUG +#include +#endif static void get_request(char *fname); static void emit_file(char* filename, struct ldap_searchobj* sop); diff --git a/dnedit.c b/dnedit.c index 21d8a69..20f93eb 100644 --- a/dnedit.c +++ b/dnedit.c @@ -120,8 +120,7 @@ int main(int argc, char *argv[] ) * * 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 ); diff --git a/doauth.c b/doauth.c index 2268ee0..35c27f6 100644 --- a/doauth.c +++ b/doauth.c @@ -76,6 +76,9 @@ post_request() 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); diff --git a/domodify.c b/domodify.c index 29f33b4..c2b1aa1 100644 --- a/domodify.c +++ b/domodify.c @@ -93,13 +93,13 @@ static void 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 @@ entry_modify_or_add( LDAP *ld, char *dn, int add, int *pwdchangedp ) 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 @@ entry_modify_or_add( LDAP *ld, char *dn, int add, int *pwdchangedp ) 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); } } } diff --git a/dosearch.c b/dosearch.c index e7679c9..3f19fb2 100644 --- a/dosearch.c +++ b/dosearch.c @@ -139,9 +139,6 @@ int main( argc, argv, env ) static void get_request(char *dn, char *ldapquery) { - int urllen = 0; - int argslen = 0; - char *p = NULL; char *ldapurl = NULL; /* diff --git a/dsgw.h b/dsgw.h index 57f2f5d..ca7d920 100644 --- a/dsgw.h +++ b/dsgw.h @@ -874,8 +874,6 @@ void dsgw_form_begin( const char* name, const char* format, ... ) #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, ... ) diff --git a/dsgwutil.c b/dsgwutil.c index f30e081..516815d 100644 --- a/dsgwutil.c +++ b/dsgwutil.c @@ -40,6 +40,7 @@ * dsgwutil.c -- misc. utility functions -- HTTP gateway */ +#include #include /* PATH_MAX */ #include "dsgw.h" #include "dbtdsgw.h" diff --git a/emitf.c b/emitf.c index cf0d80b..d963629 100644 --- a/emitf.c +++ b/emitf.c @@ -741,7 +741,8 @@ list_parse (char* slist, size_t items) 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); diff --git a/entrydisplay.c b/entrydisplay.c index db2408c..d4e2823 100644 --- a/entrydisplay.c +++ b/entrydisplay.c @@ -739,7 +739,7 @@ output_prelude_script( dsgwtmplinfo *tip ) 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 @@ emit_value( char *val, int quote_html_specials ) int freeit; if ( quote_html_specials ) { - val = dsgw_strdup_with_entities( val, &freeit ); + val = strdup_escape_entities( val, &freeit ); } else { freeit = 0; } diff --git a/htmlout.c b/htmlout.c index 3bebf86..328a855 100644 --- a/htmlout.c +++ b/htmlout.c @@ -176,7 +176,7 @@ dsgw_html_href( char *urlprefix, char *url, char *label, char *value, } } - newlabel = dsgw_strdup_with_entities( label, &freenewlabel ); + newlabel = strdup_escape_entities( label, &freenewlabel ); if ( newlabel != NULL && *newlabel != '\0' ) { dsgw_emitf( ">%s\n", newlabel ); if ( freenewlabel ) { @@ -266,98 +266,6 @@ dsgw_strcat_escaped( char *s1, const char *s2 ) } -#define DSGW_MAX_ENTITY_LEN 6 /* " */ -static char *specials = "&\"<>"; -static char *entities[] = { "&", """, "<", ">" }; -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, "" will be translated - * to "<mcs@ace.com>". - * - * 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 ("&") - back to the original unescaped string ("&") - This is necessary for converting URLs which would normally - have entities in them (e.g. search?context=foo&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 "&" - 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, ... ) { diff --git a/ldaputil.c b/ldaputil.c index a3cbef4..4fa828d 100644 --- a/ldaputil.c +++ b/ldaputil.c @@ -155,6 +155,9 @@ dsgw_init_ldap( LDAP **ldp, LDAPFiltDesc **lfdpp, int skipac, int skipauthwarnin } 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 @@ dsgw_init_ldap( LDAP **ldp, LDAPFiltDesc **lfdpp, int skipac, int skipauthwarnin #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, diff --git a/search.c b/search.c index ca1e97a..46a9033 100644 --- a/search.c +++ b/search.c @@ -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 +#endif static void get_request(char *docname); static void do_searchtype_popup( struct ldap_searchobj *sop ); diff --git a/tutor.c b/tutor.c index 30d50f4..b5e7f6c 100644 --- a/tutor.c +++ b/tutor.c @@ -198,7 +198,7 @@ main( * 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);