fix bug #64124 (IPv6 malformed)

hostname returned from zend_parse_parameters() was modified in
netsnmp_session_init() that caused imput parameter modification along with
unpredictable changes when parameter is a constant sctring.

One typo in comment was fixed.

2 files changed, 52 insertions, 11 deletions

diff --git a/ext/snmp/snmp.c b/ext/snmp/snmp.c
index 037fcce825..a3bd44c27b 100644
--- a/ext/snmp/snmp.c
+++ b/ext/snmp/snmp.c
@@ -1096,8 +1096,7 @@ static int php_snmp_parse_oid(zval *object, int st, struct objid_query *objid_qu
 static int netsnmp_session_init(php_snmp_session **session_p, int version, char *hostname, char *community, int timeout, int retries TSRMLS_DC)
 {
 	php_snmp_session *session;
-	char *pptr;
-	char buf[MAX_NAME_LEN];
+	char *pptr, *host_ptr;
 	int force_ipv6 = FALSE;
 	int n;
 	struct sockaddr **psal;
@@ -1111,8 +1110,6 @@ static int netsnmp_session_init(php_snmp_session **session_p, int version, char
 	}
 	memset(session, 0, sizeof(php_snmp_session));
 
-	strlcpy(buf, hostname, sizeof(buf));
-
 	snmp_sess_init(session);
 
 	session->version = version;
@@ -1123,13 +1120,15 @@ static int netsnmp_session_init(php_snmp_session **session_p, int version, char
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "emalloc() failed while copying hostname");
 		return (-1);
 	}
-	*(session->peername) = '\0';
+	/* we copy original hostname for further processing */
+	strlcpy(session->peername, hostname, MAX_NAME_LEN);
+	host_ptr = session->peername;
 
 	/* Reading the hostname and its optional non-default port number */
-	if (*hostname == '[') { /* IPv6 address */
+	if (*host_ptr == '[') { /* IPv6 address */
 		force_ipv6 = TRUE;
-		hostname++;
-		if ((pptr = strchr(hostname, ']'))) {
+		host_ptr++;
+		if ((pptr = strchr(host_ptr, ']'))) {
 			if (pptr[1] == ':') {
 				session->remote_port = atoi(pptr + 2);
 			}
@@ -1139,7 +1138,7 @@ static int netsnmp_session_init(php_snmp_session **session_p, int version, char
 			return (-1);
 		}
 	} else { /* IPv4 address */
-		if ((pptr = strchr(hostname, ':'))) {
+		if ((pptr = strchr(host_ptr, ':'))) {
 			session->remote_port = atoi(pptr + 1);
 			*pptr = '\0';
 		}
@@ -1147,11 +1146,13 @@ static int netsnmp_session_init(php_snmp_session **session_p, int version, char
 
 	/* since Net-SNMP library requires 'udp6:' prefix for all IPv6 addresses (in FQDN form too) we need to
 	   perform possible name resolution before running any SNMP queries */
-	if ((n = php_network_getaddresses(hostname, SOCK_DGRAM, &psal, NULL TSRMLS_CC)) == 0) { /* some resover error */
+	if ((n = php_network_getaddresses(host_ptr, SOCK_DGRAM, &psal, NULL TSRMLS_CC)) == 0) { /* some resolver error */
 		/* warnings sent, bailing out */
 		return (-1);
 	}
 
+	/* we have everything we need in psal, flush peername and fill it properly */
+	*(session->peername) = '\0';
 	res = psal;
 	while (n-- > 0) {
 		pptr = session->peername;
@@ -1181,7 +1182,7 @@ static int netsnmp_session_init(php_snmp_session **session_p, int version, char
 	}
 
 	if (strlen(session->peername) == 0) {
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown failure while resolving '%s'", buf);
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown failure while resolving '%s'", hostname);
 		return (-1);
 	}
 	/* XXX FIXME
diff --git a/ext/snmp/tests/bug64124.phpt b/ext/snmp/tests/bug64124.phpt
new file mode 100644
index 0000000000..454b06dcf1
--- /dev/null
+++ b/ext/snmp/tests/bug64124.phpt
@@ -0,0 +1,40 @@
+--TEST--                                 
+Bug #64124 IPv6 malformed
+--CREDITS--
+Boris Lytochkin
+--SKIPIF--
+<?php
+require_once(dirname(__FILE__).'/skipif.inc');
+
+$packed = str_repeat(chr(0), 15) . chr(1);
+if (@inet_ntop($packed) === false) {
+	die("skip no IPv6 support");
+}
+?>
+--FILE--
+<?php
+require_once(dirname(__FILE__).'/snmp_include.inc');
+
+# hostname variable was modified inline in netsnmp_session_init()
+# Should be checked with IPv6 since IPv4 processing code do not alter pointer position
+
+//EXPECTF format is quickprint OFF
+snmp_set_quick_print(false);
+snmp_set_valueretrieval(SNMP_VALUE_PLAIN);
+
+$checkvar = "$hostname6_port";
+
+var_dump(snmpget($checkvar, $community, '.1.3.6.1.2.1.1.1.0'));
+var_dump(($checkvar === $hostname6_port));
+var_dump(snmpget($checkvar, $community, '.1.3.6.1.2.1.1.1.0'));
+var_dump(($checkvar === $hostname6_port));
+var_dump(snmpget($checkvar, $community, '.1.3.6.1.2.1.1.1.0'));
+var_dump(($checkvar === $hostname6_port));
+?>
+--EXPECTF--
+%unicode|string%(%d) "%s"
+bool(true)
+%unicode|string%(%d) "%s"
+bool(true)
+%unicode|string%(%d) "%s"
+bool(true)