From 8950d7aa6daa60fb095079efaedeebaca10e6e00 Mon Sep 17 00:00:00 2001 From: Jonathan Giddy Date: Sun, 14 Feb 2016 08:18:14 +0000 Subject: [PATCH 1/8] Remove extra mask byte in MIME type detection --- components/net/mime_classifier.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/net/mime_classifier.rs b/components/net/mime_classifier.rs index b1ee9231baf..2e5234f0bad 100644 --- a/components/net/mime_classifier.rs +++ b/components/net/mime_classifier.rs @@ -744,7 +744,7 @@ impl ByteMatcher { TagTerminatedByteMatcher { matcher: ByteMatcher { pattern: b" Date: Fri, 19 Feb 2016 13:11:13 +0000 Subject: [PATCH 2/8] Add validation code for MIME checkers --- components/net/mime_classifier.rs | 48 +++++++++++++++++++++++++++++++ tests/unit/net/mime_classifier.rs | 6 ++++ 2 files changed, 54 insertions(+) diff --git a/components/net/mime_classifier.rs b/components/net/mime_classifier.rs index 2e5234f0bad..bcc36c3e1b9 100644 --- a/components/net/mime_classifier.rs +++ b/components/net/mime_classifier.rs @@ -150,6 +150,18 @@ impl MIMEClassifier { } } + pub fn validate(&self) -> Result<(), String> { + try!(self.image_classifier.validate()); + try!(self.audio_video_classifier.validate()); + try!(self.scriptable_classifier.validate()); + try!(self.plaintext_classifier.validate()); + try!(self.archive_classifier.validate()); + try!(self.binary_or_plaintext.validate()); + try!(self.feeds_classifier.validate()); + try!(self.font_classifier.validate()); + Ok(()) + } + //some sort of iterator over the classifiers might be better? fn sniff_unknown_type(&self, no_sniff_flag: NoSniffFlag, data: &[u8]) -> (String, String) { let should_sniff_scriptable = no_sniff_flag == NoSniffFlag::OFF; @@ -231,6 +243,8 @@ pub fn as_string_option(tup: Option<(&'static str, &'static str)>) -> Option<(St //Interface used for composite types trait MIMEChecker { fn classify(&self, data: &[u8]) -> Option<(String, String)>; + /// Validate the MIME checker configuration + fn validate(&self) -> Result<(), String>; } trait Matches { @@ -300,6 +314,15 @@ impl MIMEChecker for ByteMatcher { (self.content_type.0.to_owned(), self.content_type.1.to_owned()) }) } + + fn validate(&self) -> Result<(), String> { + if self.pattern.len() != self.mask.len() { + Err(format!("Unequal pattern and mask length for pattern {:?}", self.pattern)) + } + else { + Ok(()) + } + } } struct TagTerminatedByteMatcher { @@ -316,7 +339,12 @@ impl MIMEChecker for TagTerminatedByteMatcher { None }) } + + fn validate(&self) -> Result<(), String> { + self.matcher.validate() + } } + pub struct Mp4Matcher; impl Mp4Matcher { @@ -350,6 +378,10 @@ impl MIMEChecker for Mp4Matcher { None } } + + fn validate(&self) -> Result<(), String> { + Ok(()) + } } struct BinaryOrPlaintextClassifier; @@ -375,6 +407,11 @@ impl MIMEChecker for BinaryOrPlaintextClassifier { fn classify(&self, data: &[u8]) -> Option<(String, String)> { as_string_option(Some(self.classify_impl(data))) } + + fn validate(&self) -> Result<(), String> { + Ok(()) + } + } struct GroupedClassifier { byte_matchers: Vec>, @@ -473,6 +510,13 @@ impl MIMEChecker for GroupedClassifier { .filter_map(|matcher| matcher.classify(data)) .next() } + + fn validate(&self) -> Result<(), String> { + for byte_matcher in &self.byte_matchers { + try!(byte_matcher.validate()) + } + Ok(()) + } } enum Match { @@ -576,6 +620,10 @@ impl MIMEChecker for FeedsClassifier { fn classify(&self, data: &[u8]) -> Option<(String, String)> { as_string_option(self.classify_impl(data)) } + + fn validate(&self) -> Result<(), String> { + Ok(()) + } } //Contains hard coded byte matchers diff --git a/tests/unit/net/mime_classifier.rs b/tests/unit/net/mime_classifier.rs index bcc5b54066c..91fbb436603 100644 --- a/tests/unit/net/mime_classifier.rs +++ b/tests/unit/net/mime_classifier.rs @@ -50,6 +50,12 @@ fn test_sniff_mp4_matcher_long() { assert!(matcher.matches(&data)); } +#[test] +fn test_validate_classifier() { + let classifier = MIMEClassifier::new(); + classifier.validate().expect("Validation error") +} + #[cfg(test)] fn test_sniff_with_flags(filename_orig: &path::Path, type_string: &str, From 54b07bae916a71c69674ce356f727d1d05d272df Mon Sep 17 00:00:00 2001 From: Jonathan Giddy Date: Fri, 19 Feb 2016 13:11:41 +0000 Subject: [PATCH 3/8] Fix invalid MIME checkers --- components/net/mime_classifier.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/net/mime_classifier.rs b/components/net/mime_classifier.rs index bcc36c3e1b9..4b88850f0b4 100644 --- a/components/net/mime_classifier.rs +++ b/components/net/mime_classifier.rs @@ -678,7 +678,7 @@ impl ByteMatcher { fn image_webp() -> ByteMatcher { ByteMatcher { pattern: b"RIFF\x00\x00\x00\x00WEBPVP", - mask: b"\xFF\xFF\xFF\xFF\x00\x00\x00\x00,\xFF\xFF\xFF\xFF\xFF\xFF", + mask: b"\xFF\xFF\xFF\xFF\x00\x00\x00\x00\xFF\xFF\xFF\xFF\xFF\xFF", content_type: ("image", "webp"), leading_ignore: &[] } @@ -742,7 +742,7 @@ impl ByteMatcher { fn application_ogg() -> ByteMatcher { ByteMatcher { pattern: b"OggS", - mask: b"\xFF\xFF\xFF\xFF\xFF", + mask: b"\xFF\xFF\xFF\xFF", content_type: ("application", "ogg"), leading_ignore: &[] } @@ -992,7 +992,7 @@ impl ByteMatcher { fn application_pdf() -> ByteMatcher { ByteMatcher { pattern: b"%PDF", - mask: b"\xFF\xFF\xFF\xFF\xFF", + mask: b"\xFF\xFF\xFF\xFF", content_type: ("application", "pdf"), leading_ignore: &[] } From ab43e2b028f189e28cc7baf577b427761bdffa1b Mon Sep 17 00:00:00 2001 From: Jonathan Giddy Date: Fri, 19 Feb 2016 16:51:40 +0000 Subject: [PATCH 4/8] Ogg has a 5-byte signature, but pattern did not include NUL --- components/net/mime_classifier.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/net/mime_classifier.rs b/components/net/mime_classifier.rs index 4b88850f0b4..89f4522ac19 100644 --- a/components/net/mime_classifier.rs +++ b/components/net/mime_classifier.rs @@ -741,8 +741,8 @@ impl ByteMatcher { //The string "OggS" followed by NUL, the Ogg container signature. fn application_ogg() -> ByteMatcher { ByteMatcher { - pattern: b"OggS", - mask: b"\xFF\xFF\xFF\xFF", + pattern: b"OggS\x00", + mask: b"\xFF\xFF\xFF\xFF\xFF", content_type: ("application", "ogg"), leading_ignore: &[] } From 94f5d26131c2347efbee880b85977711ee30f231 Mon Sep 17 00:00:00 2001 From: Jonathan Giddy Date: Tue, 23 Feb 2016 10:54:54 +0000 Subject: [PATCH 5/8] Use content-type for error message --- components/net/mime_classifier.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/components/net/mime_classifier.rs b/components/net/mime_classifier.rs index 89f4522ac19..4cdc6e45ec3 100644 --- a/components/net/mime_classifier.rs +++ b/components/net/mime_classifier.rs @@ -317,7 +317,10 @@ impl MIMEChecker for ByteMatcher { fn validate(&self) -> Result<(), String> { if self.pattern.len() != self.mask.len() { - Err(format!("Unequal pattern and mask length for pattern {:?}", self.pattern)) + Err(format!( + "Unequal pattern and mask length for {}/{}", + self.content_type.0, self.content_type.1 + )) } else { Ok(()) From 480cb385fc3285eaa9b6633fb6e4486f16fd1178 Mon Sep 17 00:00:00 2001 From: Jonathan Giddy Date: Tue, 23 Feb 2016 11:52:47 +0000 Subject: [PATCH 6/8] Fix off-by-one error in MIME pattern matching This adds a size to the test webp file, since the error fixed occurs when the test object is the same length as the matched pattern, and is not equal to the pattern. --- components/net/mime_classifier.rs | 16 ++++++++++------ .../unit/net/parsable_mime/image/webp/test.webp | Bin 14 -> 14 bytes 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/components/net/mime_classifier.rs b/components/net/mime_classifier.rs index 4cdc6e45ec3..f9161c4f8a6 100644 --- a/components/net/mime_classifier.rs +++ b/components/net/mime_classifier.rs @@ -294,7 +294,7 @@ impl ByteMatcher { } else if data == self.pattern { Some(self.pattern.len()) } else { - data[..data.len() - self.pattern.len()].iter() + data[..data.len() - self.pattern.len() + 1].iter() .position(|x| !self.leading_ignore.contains(x)) .and_then(|start| if data[start..].iter() @@ -316,15 +316,19 @@ impl MIMEChecker for ByteMatcher { } fn validate(&self) -> Result<(), String> { + if self.pattern.len() == 0 { + return Err(format!( + "Zero length pattern for {}/{}", + self.content_type.0, self.content_type.1 + )) + } if self.pattern.len() != self.mask.len() { - Err(format!( - "Unequal pattern and mask length for {}/{}", + return Err(format!( + "Unequal pattern and mask length for {}/{}", self.content_type.0, self.content_type.1 )) } - else { - Ok(()) - } + Ok(()) } } diff --git a/tests/unit/net/parsable_mime/image/webp/test.webp b/tests/unit/net/parsable_mime/image/webp/test.webp index 5a907eb4ce25534893c05b85cd775bc062ce5360..ad88e62f94c95e3d5039aa9bce0579bb4e689eaf 100755 GIT binary patch literal 14 VcmWIYbaP{4U|f^b)-fUp1n6yO5< From 6ea656c6c513761694e3876368add3ee7a57ea15 Mon Sep 17 00:00:00 2001 From: Jonathan Giddy Date: Tue, 23 Feb 2016 13:46:53 +0000 Subject: [PATCH 7/8] Add check that ByteMatcher pattern has the mask applied --- components/net/mime_classifier.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/components/net/mime_classifier.rs b/components/net/mime_classifier.rs index f9161c4f8a6..d00a3251ec8 100644 --- a/components/net/mime_classifier.rs +++ b/components/net/mime_classifier.rs @@ -299,7 +299,7 @@ impl ByteMatcher { .and_then(|start| if data[start..].iter() .zip(self.pattern.iter()).zip(self.mask.iter()) - .all(|((&data, &pattern), &mask)| (data & mask) == (pattern & mask)) { + .all(|((&data, &pattern), &mask)| (data & mask) == pattern) { Some(start + self.pattern.len()) } else { None @@ -328,6 +328,14 @@ impl MIMEChecker for ByteMatcher { self.content_type.0, self.content_type.1 )) } + if self.pattern.iter().zip(self.mask.iter()).any( + |(&pattern, &mask)| pattern & mask != pattern + ) { + return Err(format!( + "Pattern not pre-masked for {}/{}", + self.content_type.0, self.content_type.1 + )) + } Ok(()) } } From 4721fd1c6a89aac101f26c0b2eb60e5dc131df27 Mon Sep 17 00:00:00 2001 From: Jonathan Giddy Date: Tue, 23 Feb 2016 14:09:56 +0000 Subject: [PATCH 8/8] Fix PDF signature to match spec --- components/net/mime_classifier.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/net/mime_classifier.rs b/components/net/mime_classifier.rs index d00a3251ec8..3b97566f018 100644 --- a/components/net/mime_classifier.rs +++ b/components/net/mime_classifier.rs @@ -1006,8 +1006,8 @@ impl ByteMatcher { //The string "%PDF-", the PDF signature. fn application_pdf() -> ByteMatcher { ByteMatcher { - pattern: b"%PDF", - mask: b"\xFF\xFF\xFF\xFF", + pattern: b"%PDF-", + mask: b"\xFF\xFF\xFF\xFF\xFF", content_type: ("application", "pdf"), leading_ignore: &[] }