Skip to content

Commit bab0b99

Browse files
committed
Detect invalid port in xp_socket parse ip address
For historical reasons, fsockopen() accepts the port and hostname separately: fsockopen('127.0.0.1', 80) However, with the introdcution of stream transports in PHP 4.3, it became possible to include the port in the hostname specifier: fsockopen('127.0.0.1:80') Or more formally: fsockopen('tcp://127.0.0.1:80') Confusing results when these two forms are combined, however. fsockopen('127.0.0.1:80', 443) results in fsockopen() attempting to connect to '127.0.0.1:80:443' which any reasonable stack would consider invalid. Unfortunately, PHP parses the address looking for the first colon (with special handling for IPv6, don't worry) and calls atoi() from there. atoi() in turn, simply stops parsing at the first non-numeric character and returns the value so far. The end result is that the explicitly supplied port is treated as ignored garbage, rather than producing an error. This diff replaces atoi() with strtol() and inspects the stop character. If additional "garbage" of any kind is found, it fails and returns an error.
1 parent 549a30d commit bab0b99

File tree

2 files changed

+55
-11
lines changed

2 files changed

+55
-11
lines changed
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
Use of double-port in fsockopen()
3+
--FILE--
4+
<?php
5+
6+
$try = [
7+
'127.0.0.1:80',
8+
'tcp://127.0.0.1:80',
9+
'[::1]:80',
10+
'tcp://[::1]:80',
11+
'localhost:80',
12+
'tcp://localhost:80',
13+
];
14+
15+
foreach ($try as $addr) {
16+
echo "== $addr ==\n";
17+
var_dump(@fsockopen($addr, 81, $errno, $errstr), $errstr);
18+
}
19+
--EXPECTF--
20+
== 127.0.0.1:80 ==
21+
bool(false)
22+
string(41) "Failed to parse address "127.0.0.1:80:81""
23+
== tcp://127.0.0.1:80 ==
24+
bool(false)
25+
string(41) "Failed to parse address "127.0.0.1:80:81""
26+
== [::1]:80 ==
27+
bool(false)
28+
string(37) "Failed to parse address "[::1]:80:81""
29+
== tcp://[::1]:80 ==
30+
bool(false)
31+
string(37) "Failed to parse address "[::1]:80:81""
32+
== localhost:80 ==
33+
bool(false)
34+
string(41) "Failed to parse address "localhost:80:81""
35+
== tcp://localhost:80 ==
36+
bool(false)
37+
string(41) "Failed to parse address "localhost:80:81""

main/streams/xp_socket.c

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -571,37 +571,44 @@ static inline char *parse_ip_address_ex(const char *str, size_t str_len, int *po
571571
char *host = NULL;
572572

573573
#ifdef HAVE_IPV6
574-
char *p;
575-
576574
if (*(str) == '[' && str_len > 1) {
577575
/* IPV6 notation to specify raw address with port (i.e. [fe80::1]:80) */
578-
p = memchr(str + 1, ']', str_len - 2);
576+
char *p = memchr(str + 1, ']', str_len - 2), *e = NULL;
579577
if (!p || *(p + 1) != ':') {
580578
if (get_err) {
581579
*err = strpprintf(0, "Failed to parse IPv6 address \"%s\"", str);
582580
}
583581
return NULL;
584582
}
585-
*portno = atoi(p + 2);
583+
*portno = strtol(p + 2, &e, 10);
584+
if (e && *e) {
585+
if (get_err) {
586+
*err = strpprintf(0, "Failed to parse address \"%s\"", str);
587+
}
588+
return NULL;
589+
}
586590
return estrndup(str + 1, p - str - 1);
587591
}
588592
#endif
593+
589594
if (str_len) {
590595
colon = memchr(str, ':', str_len - 1);
591596
} else {
592597
colon = NULL;
593598
}
599+
594600
if (colon) {
595-
*portno = atoi(colon + 1);
596-
host = estrndup(str, colon - str);
597-
} else {
598-
if (get_err) {
599-
*err = strpprintf(0, "Failed to parse address \"%s\"", str);
601+
char *e = NULL;
602+
*portno = strtol(colon + 1, &e, 10);
603+
if (!e || !*e) {
604+
return estrndup(str, colon - str);
600605
}
601-
return NULL;
602606
}
603607

604-
return host;
608+
if (get_err) {
609+
*err = strpprintf(0, "Failed to parse address \"%s\"", str);
610+
}
611+
return NULL;
605612
}
606613

607614
static inline char *parse_ip_address(php_stream_xport_param *xparam, int *portno)

0 commit comments

Comments
 (0)