Skip to content

Commit fd01e7a

Browse files
author
bors-servo
authored
Auto merge of #14623 - DominoTree:master, r=emilio
<!-- Please describe your changes on the following line: --> Add check for bad ports to http_fetch(), return NetworkError::Internal if bad port/schema combination is seen. Test added --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #14514 (github issue number if applicable). <!-- Either: --> - [x] There are tests for these changes OR <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14623) <!-- Reviewable:end -->
2 parents dd2aa41 + a56a7ba commit fd01e7a

File tree

4 files changed

+113
-0
lines changed

4 files changed

+113
-0
lines changed

components/net/fetch/methods.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,18 @@ pub fn main_fetch(request: Rc<Request>,
143143

144144
// Step 5
145145
// TODO this step (CSP port/content blocking)
146+
if let Some(port) = request.url().port() {
147+
let is_ftp = request.url().scheme() == "ftp" && (port == 20 || port == 21);
148+
static BAD_PORTS: [u16; 64] = [1, 7, 9, 11, 13, 15, 17, 19, 20, 21, 22, 23, 25, 37, 42,
149+
43, 53, 77, 79, 87, 95, 101, 102, 103, 104, 109, 110, 111,
150+
113, 115, 117, 119, 123, 135, 139, 143, 179, 389, 465, 512,
151+
513, 514, 515, 526, 530, 531, 532, 540, 556, 563, 587, 601,
152+
636, 993, 995, 2049, 3659, 4045, 6000, 6665, 6666, 6667,
153+
6668, 6669];
154+
if !is_ftp && BAD_PORTS.binary_search(&port).is_ok() {
155+
response = Some(Response::network_error(NetworkError::Internal("Request attempted on bad port".into())));
156+
}
157+
}
146158

147159
// Step 6
148160
// TODO this step (referrer policy)

tests/unit/net/fetch.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use hyper::status::StatusCode;
2323
use hyper::uri::RequestUri;
2424
use msg::constellation_msg::TEST_PIPELINE_ID;
2525
use net::fetch::cors_cache::CorsCache;
26+
use net_traits::NetworkError;
2627
use net_traits::ReferrerPolicy;
2728
use net_traits::request::{Origin, RedirectMode, Referrer, Request, RequestMode};
2829
use net_traits::response::{CacheState, Response, ResponseBody, ResponseType};
@@ -59,6 +60,18 @@ fn test_fetch_response_is_not_network_error() {
5960
}
6061
}
6162

63+
#[test]
64+
fn test_fetch_on_bad_port_is_network_error() {
65+
let url = ServoUrl::parse("http://www.example.org:6667").unwrap();
66+
let origin = Origin::Origin(url.origin());
67+
let request = Request::new(url, Some(origin), false, None);
68+
*request.referrer.borrow_mut() = Referrer::NoReferrer;
69+
let fetch_response = fetch(request, None);
70+
assert!(fetch_response.is_network_error());
71+
let fetch_error = fetch_response.get_network_error().unwrap();
72+
assert!(fetch_error == &NetworkError::Internal("Request attempted on bad port".into()))
73+
}
74+
6275
#[test]
6376
fn test_fetch_response_body_matches_const_message() {
6477
static MESSAGE: &'static [u8] = b"Hello World!";

tests/wpt/metadata/MANIFEST.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39725,6 +39725,12 @@
3972539725
"url": "/cssom/shorthand-serialization.html"
3972639726
}
3972739727
],
39728+
"fetch/api/request/request-bad-port.html": [
39729+
{
39730+
"path": "fetch/api/request/request-bad-port.html",
39731+
"url": "/fetch/api/request/request-bad-port.html"
39732+
}
39733+
],
3972839734
"html/semantics/forms/form-submission-0/submit-entity-body.html": [
3972939735
{
3973039736
"path": "html/semantics/forms/form-submission-0/submit-entity-body.html",
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<!doctype html>
2+
<meta charset="utf-8">
3+
<title></title>
4+
<script src="/resources/testharness.js"></script>
5+
<script src="/resources/testharnessreport.js"></script>
6+
<script>
7+
8+
// list of bad ports according to
9+
// https://fetch.spec.whatwg.org/#port-blocking
10+
var BLOCKED_PORTS_LIST = [
11+
1, // tcpmux
12+
7, // echo
13+
9, // discard
14+
11, // systat
15+
13, // daytime
16+
15, // netstat
17+
17, // qotd
18+
19, // chargen
19+
20, // ftp-data
20+
21, // ftp
21+
22, // ssh
22+
23, // telnet
23+
25, // smtp
24+
37, // time
25+
42, // name
26+
43, // nicname
27+
53, // domain
28+
77, // priv-rjs
29+
79, // finger
30+
87, // ttylink
31+
95, // supdup
32+
101, // hostriame
33+
102, // iso-tsap
34+
103, // gppitnp
35+
104, // acr-nema
36+
109, // pop2
37+
110, // pop3
38+
111, // sunrpc
39+
113, // auth
40+
115, // sftp
41+
117, // uucp-path
42+
119, // nntp
43+
123, // ntp
44+
135, // loc-srv / epmap
45+
139, // netbios
46+
143, // imap2
47+
179, // bgp
48+
389, // ldap
49+
465, // smtp+ssl
50+
512, // print / exec
51+
513, // login
52+
514, // shell
53+
515, // printer
54+
526, // tempo
55+
530, // courier
56+
531, // chat
57+
532, // netnews
58+
540, // uucp
59+
556, // remotefs
60+
563, // nntp+ssl
61+
587, // smtp
62+
601, // syslog-conn
63+
636, // ldap+ssl
64+
993, // imap+ssl
65+
995, // pop3+ssl
66+
2049, // nfs
67+
3659, // apple-sasl
68+
4045, // lockd
69+
6000, // x11
70+
6665, // irc (alternate)
71+
6666, // irc (alternate)
72+
6667, // irc (default)
73+
6668, // irc (alternate)
74+
6669, // irc (alternate)
75+
];
76+
77+
BLOCKED_PORTS_LIST.map(function(a){
78+
promise_test(function(t){
79+
return promise_rejects(t, new TypeError(), fetch("http://example.com:" + a))
80+
}, 'Request on bad port ' + a + ' should throw TypeError.');
81+
});
82+
</script>

0 commit comments

Comments
 (0)