Merge pull request #1 from keplernetwork/fix-CVE-2019-9195

Fixes CVE-2019-9195

chain/Cargo.toml

@@ -21,6 +21,8 @@ serde = "1"
 serde_derive = "1"
 chrono = "0.4.4"
 lru-cache = "0.1"
+lazy_static = "1"
+regex = "1"
 
 kepler_core = { path = "../core", version = "1.0.1" }
 kepler_keychain = { path = "../keychain", version = "1.0.1" }

chain/src/txhashset/txhashset.rs

@@ -1426,11 +1426,28 @@ pub fn zip_write(
 ) -> Result<(), Error> {
 	let txhashset_path = Path::new(&root_dir).join(TXHASHSET_SUBDIR);
 	fs::create_dir_all(txhashset_path.clone())?;
-	zip::decompress(txhashset_data, &txhashset_path)
+	zip::decompress(txhashset_data, &txhashset_path, expected_file)
 		.map_err(|ze| ErrorKind::Other(ze.to_string()))?;
 	check_and_remove_files(&txhashset_path, header)
 }
 
+fn expected_file(path: &Path) -> bool {
+	use lazy_static::lazy_static;
+	use regex::Regex;
+	let s_path = path.to_str().unwrap_or_else(|| "");
+	lazy_static! {
+		static ref RE: Regex = Regex::new(
+			format!(
+				r#"^({}|{}|{})(/pmmr_(hash|data|leaf|prun)\.bin(\.\w*)?)?$"#,
+				OUTPUT_SUBDIR, KERNEL_SUBDIR, RANGE_PROOF_SUBDIR
+			)
+			.as_str()
+		)
+		.unwrap();
+	}
+	RE.is_match(&s_path)
+}
+
 /// Check a txhashset directory and remove any unexpected
 fn check_and_remove_files(txhashset_path: &PathBuf, header: &BlockHeader) -> Result<(), Error> {
 	// First compare the subdirectories
@@ -1566,3 +1583,23 @@ pub fn input_pos_to_rewind(
 	let bitmap = bitmap_fast_or(None, &mut block_input_bitmaps).unwrap();
 	Ok(bitmap)
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn test_expected_files() {
+		assert!(!expected_file(Path::new("kernels")));
+		assert!(!expected_file(Path::new("xkernel")));
+		assert!(expected_file(Path::new("kernel")));
+		assert!(expected_file(Path::new("kernel/pmmr_data.bin")));
+		assert!(expected_file(Path::new("kernel/pmmr_hash.bin")));
+		assert!(expected_file(Path::new("kernel/pmmr_leaf.bin")));
+		assert!(expected_file(Path::new("kernel/pmmr_prun.bin")));
+		assert!(expected_file(Path::new("kernel/pmmr_leaf.bin.deadbeef")));
+		assert!(!expected_file(Path::new("xkernel/pmmr_data.bin")));
+		assert!(!expected_file(Path::new("kernel/pmmrx_data.bin")));
+		assert!(!expected_file(Path::new("kernel/pmmr_data.binx")));
+	}
+}

chain/tests/test_txhashset.rs

@@ -136,9 +136,5 @@ fn txhashset_contains_expected_files(dirname: String, path_buf: PathBuf) -> bool
 	let intersection: HashSet<_> = zip_files_hashset
 		.difference(&expected_files_hashset)
 		.collect();
-	if intersection.is_empty() {
-		true
-	} else {
-		false
-	}
+	intersection.is_empty()
 }

util/src/zip.rs

@@ -62,15 +62,22 @@ pub fn compress(src_dir: &Path, dst_file: &File) -> ZipResult<()> {
 }
 
 /// Decompress a source file into the provided destination path.
-pub fn decompress<R>(src_file: R, dest: &Path) -> ZipResult<()>
+pub fn decompress<R, F>(src_file: R, dest: &Path, expected: F) -> ZipResult<usize>
 where
 	R: io::Read + io::Seek,
+	F: Fn(&Path) -> bool,
 {
+	let mut decompressed = 0;
 	let mut archive = zip_rs::ZipArchive::new(src_file)?;
 
 	for i in 0..archive.len() {
 		let mut file = archive.by_index(i)?;
-		let file_path = dest.join(file.name());
+		let san_name = file.sanitized_name();
+		if san_name.to_str().unwrap_or("") != file.name() || !expected(&san_name) {
+			info!("ignoring a suspicious file: {}", file.name());
+			continue;
+		}
+		let file_path = dest.join(san_name);
 
 		if (&*file.name()).ends_with('/') {
 			fs::create_dir_all(&file_path)?;
@@ -80,7 +87,6 @@ where
 					fs::create_dir_all(&p)?;
 				}
 			}
-			//let mut outfile = fs::File::create(&file_path)?;
 			let res = fs::File::create(&file_path);
 			let mut outfile = match res {
 				Err(e) => {
@@ -90,6 +96,7 @@ where
 				Ok(r) => r,
 			};
 			io::copy(&mut file, &mut outfile)?;
+			decompressed += 1;
 		}
 
 		// Get and Set permissions
@@ -104,5 +111,5 @@ where
 			}
 		}
 	}
-	Ok(())
+	Ok(decompressed)
 }

util/tests/zip.rs

@@ -38,14 +38,22 @@ fn zip_unzip() {
 
 	fs::create_dir_all(root.join("./dezipped")).unwrap();
 	let zip_file = File::open(zip_name).unwrap();
-	zip::decompress(zip_file, &root.join("./dezipped")).unwrap();
+	zip::decompress(zip_file, &root.join("./dezipped"), |_| true).unwrap();
 
 	assert!(root.join("to_zip/foo.txt").is_file());
 	assert!(root.join("to_zip/bar.txt").is_file());
 	assert!(root.join("to_zip/sub").is_dir());
 	let lorem = root.join("to_zip/sub/lorem");
 	assert!(lorem.is_file());
 	assert!(lorem.metadata().unwrap().len() == 55);
+
+	let decompressed = zip::decompress(
+		File::open("tests/test.zip").unwrap(),
+		&root.join("./dezipped"),
+		|_| true,
+	)
+	.unwrap();
+	assert_eq!(decompressed, 1);
 }
 
 fn write_files(dir_name: String, root: &Path) -> io::Result<()> {