Re: core in libspf2 on SunOS 5.8
"Manish Raje" <[email protected]> Mon, 28 Jul 2008 20:44:16 +0530
| Newsgroups | gmane.mail.spam.spf.devel |
|---|---|
| Message-ID | <[email protected]> |
Manish Raje <manish.raje <at> gmail.com> writes: > > > Hi > > I'm using libspf2 version 1.0.4. > I'm seeing core dumps with stack trace as - > =>[1] __ns_name_skip(0xfb5ef22c, 0xfb5f87be, 0x1ce, 0x0, 0x0, 0xce), at 0xfe5d1bc8 [2] dn_skipname(0xfb5f77b0, 0xfb5f87be, 0xfb5f0508, 0x3, 0x29a5bac, 0xfb5f03f0), at 0xfe5d8e10 [3] __ns_skiprr(0xfb5efc1e, 0xfb5f87be, 0x1, 0xfb5f87be, 0x4f4, 0xfb5f77b0), at 0xfe5d1f54 > [4] __ns_initparse(0xfb5f87be, 0xfb5efbbc, 0xfb5efbbc, 0x1, 0xfb5efbb8, 0xfb5efc1e), at 0xfe5d2124 [5] SPF_dns_lookup_resolv(0x20a0378, 0xfb5f0490, 0xc, 0x1, 0xfd1e8720, 0xdb), at 0xfd1e8b4c [6] SPF_dns_rlookup(0x20a0378, 0xfb5f05b4, 0xc, 0x1, 0x20, 0x20), at 0xfd1e5160 > [7] SPF_eval_id(0x4d1298, 0x4c2a18, 0x20a0378, 0x1, 0x0, 0x0), at 0xfd1ebf88 [8] SPF_result(0x4d1298, 0x20a0378, 0x0, 0xfb5f0988, 0x4cbb9c, 0x1c00), at 0xfd1e72a8 > > On further debugging, it looks like, there is an issue with the code in the function 'SPF_dns_lookup_resolv' (spf_dns_resolv.c). > The code that calls res_query is - > /* * try resolving the name */#if HAVE_DECL_RES_NINIT dns_len = res_nquery( &spfhook->res_state, domain, ns_c_in, rr_type, response, sizeof( response ) ); > #else dns_len = res_query( domain, ns_c_in, rr_type, response, sizeof( response ) );#endif > The Solaris manpage says - > The res_nquery() and res_query() routines return a lengththat may be **bigger** than anslen. In that case, retry thequery with a larger buf. The answer to the second query maybe larger still], so it is recommended that you supply a buf > larger than the answer returned by the previous query.answer must be large enough to receive a maximum UDPresponse from the server or parts of the answer will besilently discarded. The default maximum UDP response size > is 512 bytes. > > In this core dump I saw that dns_len was greater than response size. response in code is response[2048]. Whereas, dns_len was set to 50K plus. Calling res_nquery with this faulty length caused the core dump. > > Has anyone experienced this issue before ? > Any plans to fix this ? I checked version 1.2.5 and the code is same. So the issue would exist in latest code as well. > > Could anyone point me to the bug database please ? > > Thanks, > Manish. > Hi All Me and Mike (Zraly) have worked around this issue by modifying the spf code in "spf_dns_resolv.c". It would be great if you could incorporate the changes in upcoming versions of SPF. Changes attached. Thanks, Manish.
changes_for_spf_dns_resolv.txt
(text/plain, 9.3 KB)
+typedef struct dynamic_buffer
+{
+ size_t len;
+ u_char *respBuf;
+} dynamic_buffer_t;
static inline SPF_dns_resolv_config_t *SPF_voidp2spfhook( void *hook )
{ return (SPF_dns_resolv_config_t *)hook; }
static inline void *SPF_spfhook2voidp( SPF_dns_resolv_config_t *spfhook )
{ return (void *)spfhook; }
+#define INITIAL_SIZE 2048
-static SPF_dns_rr_t *SPF_dns_lookup_resolv( SPF_dns_config_t spfdcid, const
char *domain, ns_type rr_type, int should_cache )
+static void dynamic_buffer_init(dynamic_buffer_t *bufp)
+{
+ bufp->respBuf = (u_char *) malloc(sizeof(u_char) * INITIAL_SIZE);
+ if (!bufp->respBuf) {
+ bufp->len = -1;
+ return;
+ }
+
+ /* All went well. Set the initial size of buffer */
+ bufp->len = INITIAL_SIZE;
+}
+
+static void dynamic_buffer_free(dynamic_buffer_t *bufp)
+{
+ /* Get rid of dynamically allocated memory */
+ if (bufp->respBuf)
+ {
+ free(bufp->respBuf);
+ bufp->respBuf = NULL;
+ }
+}
+
+static u_char *dynamic_buffer_grow(dynamic_buffer_t *bufp, size_t sz)
+{
+ u_char *newBuf = (u_char *) realloc(bufp->respBuf, sz);
+
+ if (newBuf == NULL)
+ return NULL; /* leave (bufp->respBuf unchanged */
+
+ /* Record the newly allocated pointer in original struct pointer */
+ bufp->respBuf = newBuf;
+
+ /* Record the new buffer length */
+ bufp->len = sz;
+
+ return bufp->respBuf;
+}
+
+#define MAX_TRIES 3
+
+static SPF_dns_rr_t *SPF_dns_lookup_resolv_internal( SPF_dns_config_t spfdcid,
const char *domain, ns_type rr_type, int should_cache, dynamic_buffer_t *dynBuf )
{
SPF_dns_iconfig_t *spfdic = SPF_dcid2spfdic( spfdcid );
SPF_dns_resolv_config_t *spfhook = SPF_voidp2spfhook( spfdic->hook );
SPF_dns_rr_t *spfrr;
int err;
int i;
int nrec;
int cnt;
- u_char response[2048];
+ u_char *response = dynBuf->respBuf;
+ int max_try_ctr = 0;
int dns_len;
ns_msg ns_handle;
ns_rr rr;
@@ -142,19 +189,21 @@ Prev Next static SPF_dns_rr_t *SPF_dns_lookup_reso
(rr_type == ns_t_any) ? "ANY" :
"??" ),
rr_type );
+ for (max_try_ctr = 0; max_try_ctr < MAX_TRIES; max_try_ctr++)
+ {
/*
* try resolving the name
*/
#if HAVE_DECL_RES_NINIT
dns_len = res_nquery( &spfhook->res_state, domain, ns_c_in, rr_type,
- response, sizeof( response ) );
+ response, dynBuf->len );
#else
dns_len = res_query( domain, ns_c_in, rr_type,
- response, sizeof( response ) );
+ response, dynBuf->len );
#endif
if ( dns_len < 0 )
{
if ( spfhook->debug )
@@ -165,13 +214,33 @@ Prev Next static SPF_dns_rr_t *SPF_dns_lookup_reso
return SPF_dcid2spfdic( spfdic->layer_below )->lookup(
spfdic->layer_below, domain, rr_type, should_cache );
spfrr->herrno = SPF_h_errno;
return spfrr;
}
+ else if ( dns_len > dynBuf->len)
+ {
+ if ( spfhook->debug )
+ SPF_debugf( "dns response length = %d, greater than buffer
+ size = %d", dns_len, dynBuf->len);
+
+ /* realloc the buffer to new size */
+ if ((response = dynamic_buffer_grow(dynBuf, dns_len)) == NULL) {
+ if ( spfhook->debug )
+ SPF_debugf("realloc failed!");
+
+ return spfrr;
+ }
+
+ /* loop again with a new buffer of size = dns_len */
+ }
else
+ {
+ /* Everything OK. Break out of the loop */
spfrr->herrno = NETDB_SUCCESS;
-
+ break;
+ }
+ } /* end for */
err = ns_initparse( response, dns_len, &ns_handle );
if ( err < 0 ) /* 0 or -1 */
{
@@ -246,11 +315,11 @@ Prev Next static SPF_dns_rr_t *SPF_dns_lookup_reso
ip6_buf, sizeof( ip6_buf ) ));
break;
case ns_t_ns:
err = ns_name_uncompress( response,
- response + sizeof( response ),
+ response + dynBuf->len,
rdata,
name_buf, sizeof( name_buf ) );
if ( err < 0 ) /* 0 or -1 */
{
SPF_debugf( "ns_name_uncompress failed: err = %d %s (%d)",
@@ -260,11 +329,11 @@ Prev Next static SPF_dns_rr_t *SPF_dns_lookup_reso
SPF_debugf( "NS: %s", name_buf );
break;
case ns_t_cname:
err = ns_name_uncompress( response,
- response + sizeof( response ),
+ response + dynBuf->len,
rdata,
name_buf, sizeof( name_buf ) );
if ( err < 0 ) /* 0 or -1 */
{
SPF_debugf( "ns_name_uncompress failed: err = %d %s (%d)",
@@ -275,11 +344,11 @@ Prev Next static SPF_dns_rr_t *SPF_dns_lookup_reso
break;
case ns_t_mx:
prio = ns_get16( rdata );
err = ns_name_uncompress( response,
- response + sizeof( response ),
+ response + dynBuf->len,
rdata + NS_INT16SZ,
name_buf, sizeof( name_buf ) );
if ( err < 0 ) /* 0 or -1 */
{
SPF_debugf( "ns_name_uncompress failed: err = %d %s (%d)",
@@ -295,11 +364,11 @@ Prev Next static SPF_dns_rr_t *SPF_dns_lookup_reso
rdlen, rdlen-1, rdata+1 );
break;
case ns_t_ptr:
err = ns_name_uncompress( response,
- response + sizeof( response ),
+ response + dynBuf->len,
rdata,
name_buf, sizeof( name_buf ) );
if ( err < 0 ) /* 0 or -1 */
{
SPF_debugf( "ns_name_uncompress failed: err = %d %s (%d)",
@@ -353,11 +422,11 @@ Prev Next static SPF_dns_rr_t *SPF_dns_lookup_reso
/* FIXME: are CNAMEs always sent with the real RR? */
break;
case ns_t_mx:
err = ns_name_uncompress( response,
- response + sizeof( response ),
+ response + dynBuf->len,
rdata + NS_INT16SZ,
name_buf, sizeof( name_buf ) );
if ( err < 0 ) /* 0 or -1 */
{
if ( spfhook->debug > 1 )
@@ -405,11 +474,11 @@ Prev Next static SPF_dns_rr_t *SPF_dns_lookup_reso
cnt++;
break;
case ns_t_ptr:
err = ns_name_uncompress( response,
- response + sizeof( response ),
+ response + dynBuf->len,
rdata,
name_buf, sizeof( name_buf ) );
if ( err < 0 ) /* 0 or -1 */
{
if ( spfhook->debug > 1 )
@@ -438,10 +507,42 @@ Prev Next static SPF_dns_rr_t *SPF_dns_lookup_reso
spfhook->spfrr.herrno = NO_DATA;
return spfrr;
}
+static SPF_dns_rr_t *SPF_dns_lookup_resolv( SPF_dns_config_t spfdcid, const
char *domain, ns_type rr_type, int should_cache )
+{
+ SPF_dns_iconfig_t *spfdic;
+ SPF_dns_resolv_config_t *spfhook;
+ SPF_dns_rr_t *ret_val;
+ dynamic_buffer_t dynBuf;
+
+ /* Initialize the dynamic buffer within the structure */
+ dynamic_buffer_init(&dynBuf);
+
+ /* Initialization of dynBuf failed. Return error */
+ if (-1 == dynBuf.len)
+ {
+ spfdic = SPF_dcid2spfdic( spfdcid );
+ spfhook = SPF_voidp2spfhook( spfdic->hook );
+ ret_val = &spfhook->spfrr;
+ SPF_dns_reset_rr( ret_val );
+ ret_val->herrno = NO_RECOVERY;
+ ret_val->rr_type = rr_type;
+
+ return ret_val;
+ }
+
+ /* Query DNS and parse response */
+ ret_val = SPF_dns_lookup_resolv_internal(spfdcid, domain, rr_type,
should_cache, &dynBuf);
+
+ /* Free the dynamic buffer */
+ dynamic_buffer_free(&dynBuf);
+
+ return ret_val;
+
+}
SPF_dns_config_t SPF_dns_create_config_resolv2( SPF_dns_config_t layer_below,
int debug, int timeout, int retry )
{
SPF_dns_iconfig_t *spfdic;
SPF_dns_resolv_config_t *spfhook;