From 1be5683de1e1d35e7d8c12ed7bd55ede31ccb7e3 Mon Sep 17 00:00:00 2001 From: "psyc://psyced.org/~lynX" <@> Date: Sat, 28 Nov 2009 13:43:46 +0100 Subject: [PATCH] tommie found some buffer overflows in erq --- CHANGELOG => CHANGELOG-psyclpc | 0 src/pkg-idna.c | 9 +-------- src/string_spec | 5 ----- src/util/erq/erq.c | 29 ++++++++++++++++++++++++++--- src/version.sh | 2 +- 5 files changed, 28 insertions(+), 17 deletions(-) rename CHANGELOG => CHANGELOG-psyclpc (100%) diff --git a/CHANGELOG b/CHANGELOG-psyclpc similarity index 100% rename from CHANGELOG rename to CHANGELOG-psyclpc diff --git a/src/pkg-idna.c b/src/pkg-idna.c index e54e3a4..9bf7f4f 100644 --- a/src/pkg-idna.c +++ b/src/pkg-idna.c @@ -117,13 +117,6 @@ f_idna_stringprep (svalue_t *sp) * profile is one of the stringprep profiles defined in ldmuds idn.h * str is assumed to be in utf-8 charset (see convert_charset) * flags is one of the stringprep flags defined in LDMud's idn.h . - * - * funny plan to use it only for comparison: - * maybe we should modify ldmuds string structure so that applying - * stringprep will create a pointer to the normalized form which - * is then used for string comparisons rather than the non-normalized - * form - * */ { char *buf; @@ -216,7 +209,7 @@ f_idna_stringprep (svalue_t *sp) { errorf("stringprep(): Error %s", stringprep_strerror(ret)); /* NOTREACHED */ - } + } else { // free the string argument diff --git a/src/string_spec b/src/string_spec index 443d81c..b90dd1b 100644 --- a/src/string_spec +++ b/src/string_spec @@ -49,7 +49,6 @@ PERCENT "%" PERCENT_S "%s" PERIOD "." PGSQL "pgsql" -LDAP "ldap" PROG_DEALLOCATED "program deallocated" PTMALLOC "ptmalloc" RESERVE "reserve" @@ -236,8 +235,4 @@ SQLITE_OPEN "sl_open" SQLITE_PRAGMA "sqlite_pragma" #endif -#ifdef USE_EXPAT - -#endif - /***************************************************************************/ diff --git a/src/util/erq/erq.c b/src/util/erq/erq.c index 6812cc5..36a89c2 100644 --- a/src/util/erq/erq.c +++ b/src/util/erq/erq.c @@ -146,6 +146,11 @@ typedef int length_t; #define MAX_REPLY ERQ_MAX_REPLY +/* don't know about the rfcs but here + * hostnames are limited to 128 chars --lynX */ +#define MAX_HOSTNAME 128 + + #if MAX_REPLY >= ERQ_BUFSIZE # undef MAX_REPLY # define MAX_REPLY (ERQ_BUFSIZE - 1) @@ -819,7 +824,9 @@ start_subserver (long server_num, long seed) { union ticket_u ticket; - char header[16]; + /* actually, this has always been used as outgoing buffer. + * surprising, it didn't crash! */ + char header[MAX_HOSTNAME + 16]; long num, msglen; int request; pid_t child = 0; @@ -1004,7 +1011,7 @@ start_subserver (long server_num, long seed) if (msglen > sizeof(buf)) { /* Prevent a buffer overflow */ - fprintf(stderr, "%s ERROR: Read %ld > buffer size %ld\n", time_stamp(), msglen, sizeof(buf)); + fprintf(stderr, "%s ERROR: Read %ld > buffer size %ld\n", time_stamp(), msglen, (long) sizeof(buf)); num = readn(0, buf, sizeof(buf)); /* Discard the rest of the input from the channel */ @@ -1079,6 +1086,14 @@ start_subserver (long server_num, long seed) struct hostent *hp; + if (msglen >= MAX_HOSTNAME) { +#if 1 || ERQ_DEBUG > 0 + msglen = MAX_HOSTNAME-1; + buf[msglen] = '\0'; + fprintf(stderr, "%s: ERQ_LOOKUP dropping too long hostname '%s'\n", time_stamp(), buf); +#endif + break; + } /* handle stays in header[4..7] */ header[8] = CHILD_FREE; memcpy(header+9, buf, msglen); /* copy address */ @@ -1200,6 +1215,14 @@ start_subserver (long server_num, long seed) int counter; char srvbuf[MAX_REPLY]; + if (msglen >= MAX_HOSTNAME) { +#if 1 || ERQ_DEBUG > 0 + msglen = MAX_HOSTNAME-1; + buf[msglen] = '\0'; + fprintf(stderr, "%s: ERQ_LOOKUP_SRV dropping too long hostname '%s'\n", time_stamp(), buf); +#endif + break; + } header[8] = CHILD_FREE; counter = 0; srvbuf[0] = '\0'; @@ -1543,7 +1566,7 @@ die: memcpy(header+4, child_handle, 4); header[9] = ERQ_E_UNKNOWN; header[10] = 0; - write(1, header, 11); /* releases handle */ + write1(header, 11); /* releases handle */ } #if ERQ_DEBUG > 0 diff --git a/src/version.sh b/src/version.sh index 809eeba..340c55d 100644 --- a/src/version.sh +++ b/src/version.sh @@ -17,7 +17,7 @@ version_longtype="stable" # A timestamp, to be used by bumpversion and other scripts. # It can be used, for example, to 'touch' this file on every build, thus # forcing revision control systems to add it on every checkin automatically. -version_stamp="Thu May 21 23:49:43 CEST 2009" +version_stamp="Fri May 22 11:28:32 CEST 2009" # Okay, LDMUD is using 3.x.x so to avoid conflicts let's just use 4.x.x version_major=4