From 3f9135430f3382bfc7ffbad360c2e0c99b41bf4f Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Thu, 30 Nov 2023 15:25:52 +0100 Subject: [PATCH] change unwrap to ok_or or unwrap_or Replace a plain `unwrap()` with `ok_or` or `unwrap_or` to prevent from binaries or libs from panicking at runtime. --- omaha/src/response.rs | 35 ++++++++++++++++++-------- src/bin/download_sysext.rs | 2 +- test/crau_verify.rs | 3 ++- update-format-crau/src/delta_update.rs | 16 ++++++------ update-format-crau/src/verify_sig.rs | 20 +++++++++++---- 5 files changed, 51 insertions(+), 25 deletions(-) diff --git a/omaha/src/response.rs b/omaha/src/response.rs index 978769a..8c41ace 100644 --- a/omaha/src/response.rs +++ b/omaha/src/response.rs @@ -149,8 +149,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for Manifest<'a> { } } - if let Token::ElementEnd { end: ElementEnd::Empty, .. } - = reader.next().unwrap()? + if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. }) + = reader.next().ok_or(XmlError::MissingField { + name: "Manifest".to_owned(), + field: "version".to_owned(), + })? { return Ok(Manifest { version: __self_version @@ -170,8 +173,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for Manifest<'a> { while (reader.find_attribute()?).is_some() {} - if let Token::ElementEnd { end: ElementEnd::Empty, .. } - = reader.next().unwrap()? + if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. }) + = reader.next().ok_or(XmlError::MissingField { + name: "Manifest".to_owned(), + field: "version".to_owned(), + })? { continue; } @@ -197,8 +203,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for Manifest<'a> { while (reader.find_attribute()?).is_some() {} - if let Token::ElementEnd { end: ElementEnd::Empty, .. } - = reader.next().unwrap()? + if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. }) + = reader.next().ok_or(XmlError::MissingField { + name: "Manifest".to_owned(), + field: "version".to_owned(), + })? { continue; } @@ -264,8 +273,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for UpdateCheck<'a> { } } - if let Token::ElementEnd { end: ElementEnd::Empty, .. } - = reader.next().unwrap()? + if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. }) + = reader.next().ok_or(XmlError::MissingField { + name: "UpdateCheck".to_owned(), + field: "manifest".to_owned(), + })? { return Ok(UpdateCheck { status: __self_status @@ -288,8 +300,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for UpdateCheck<'a> { reader.read_till_element_start("urls")?; while (reader.find_attribute()?).is_some() {} - if let Token::ElementEnd { end: ElementEnd::Empty, .. } - = reader.next().unwrap()? + if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. }) + = reader.next().ok_or(XmlError::MissingField { + name: "UpdateCheck".to_owned(), + field: "manifest".to_owned(), + })? { continue; } diff --git a/src/bin/download_sysext.rs b/src/bin/download_sysext.rs index 000b014..905a496 100644 --- a/src/bin/download_sysext.rs +++ b/src/bin/download_sysext.rs @@ -154,7 +154,7 @@ impl<'a> Package<'a> { let datablobspath = tmpdir.join("ue_data_blobs"); // Get length of header and data, including header and manifest. - let header_data_length = delta_update::get_header_data_length(&header, &delta_archive_manifest); + let header_data_length = delta_update::get_header_data_length(&header, &delta_archive_manifest).context("failed to get header data length")?; let hdhash = self.hash_on_disk(from_path, Some(header_data_length)).context(format!("failed to hash_on_disk path ({:?}) failed", from_path.display()))?; let hdhashvec: Vec = hdhash.clone().into(); diff --git a/test/crau_verify.rs b/test/crau_verify.rs index e6fbb54..32a242a 100644 --- a/test/crau_verify.rs +++ b/test/crau_verify.rs @@ -4,6 +4,7 @@ use std::fs; use update_format_crau::delta_update; +use anyhow::Context; use argh::FromArgs; const PUBKEY_FILE: &str = "../src/testdata/public_key_test_pkcs8.pem"; @@ -43,7 +44,7 @@ fn main() -> Result<(), Box> { let headerdatapath = tmpdir.join("ue_header_data"); // Get length of header and data, including header and manifest. - let header_data_length = delta_update::get_header_data_length(&header, &delta_archive_manifest); + let header_data_length = delta_update::get_header_data_length(&header, &delta_archive_manifest).context("failed to get header data length")?; let hdhash = ue_rs::hash_on_disk_sha256(headerdatapath.as_path(), Some(header_data_length))?; let hdhashvec: Vec = hdhash.clone().into(); diff --git a/update-format-crau/src/delta_update.rs b/update-format-crau/src/delta_update.rs index 352b359..7a94a26 100644 --- a/update-format-crau/src/delta_update.rs +++ b/update-format-crau/src/delta_update.rs @@ -4,7 +4,7 @@ use std::fs::File; use std::path::Path; use log::{debug, info}; use bzip2::read::BzDecoder; -use anyhow::{Context, Result, bail}; +use anyhow::{Context, Result, anyhow, bail}; use protobuf::Message; @@ -91,11 +91,11 @@ pub fn get_signatures_bytes<'a>(f: &'a mut BufReader, header: &'a DeltaUpd _ => None, }; - Ok(signatures_bytes.unwrap()) + signatures_bytes.ok_or(anyhow!("failed to get signature bytes slice")) } // Return data length, including header and manifest. -pub fn get_header_data_length(header: &DeltaUpdateFileHeader, manifest: &proto::DeltaArchiveManifest) -> usize { +pub fn get_header_data_length(header: &DeltaUpdateFileHeader, manifest: &proto::DeltaArchiveManifest) -> Result { // Read from the beginning of the stream, which means the whole buffer including // delta update header as well as manifest. That is because data that must be verified // with signatures start from the beginning. @@ -103,13 +103,13 @@ pub fn get_header_data_length(header: &DeltaUpdateFileHeader, manifest: &proto:: // Payload data structure: // | header | manifest | data blobs | signatures | - header.translate_offset(manifest.signatures_offset.unwrap()) as usize + Ok(header.translate_offset(manifest.signatures_offset.ok_or(anyhow!("no signature offset"))?) as usize) } // Take a buffer reader, delta file header, manifest as input. // Return path to data blobs, without header, manifest, or signatures. pub fn get_data_blobs<'a>(f: &'a mut BufReader, header: &'a DeltaUpdateFileHeader, manifest: &proto::DeltaArchiveManifest, tmpfile: &Path) -> Result { - let tmpdir = tmpfile.parent().unwrap(); + let tmpdir = tmpfile.parent().ok_or(anyhow!("unable to get parent directory"))?; fs::create_dir_all(tmpdir).context(format!("failed to create directory {:?}", tmpdir))?; let mut outfile = File::create(tmpfile).context(format!("failed to create file {:?}", tmpfile))?; @@ -118,8 +118,8 @@ pub fn get_data_blobs<'a>(f: &'a mut BufReader, header: &'a DeltaUpdateFil // get_header_data_length. // Iterate each partition_operations to get data offset and data length. for pop in &manifest.partition_operations { - let data_offset = pop.data_offset.unwrap(); - let data_length = pop.data_length.unwrap(); + let data_offset = pop.data_offset.ok_or(anyhow!("unable to get data offset"))?; + let data_length = pop.data_length.ok_or(anyhow!("unable to get data length"))?; let mut partdata = vec![0u8; data_length as usize]; @@ -128,7 +128,7 @@ pub fn get_data_blobs<'a>(f: &'a mut BufReader, header: &'a DeltaUpdateFil f.read_exact(&mut partdata).context(format!("failed to read data with length {:?}", data_length))?; // In case of bzip2-compressed chunks, extract. - if pop.type_.unwrap() == proto::install_operation::Type::REPLACE_BZ.into() { + if pop.type_.ok_or(anyhow!("unable to get type_ from partition operations"))? == proto::install_operation::Type::REPLACE_BZ.into() { let mut bzdecoder = BzDecoder::new(&partdata[..]); let mut partdata_unpacked = Vec::new(); bzdecoder.read_to_end(&mut partdata_unpacked).context(format!("failed to unpack bzip2ed data at offset {:?}", translated_offset))?; diff --git a/update-format-crau/src/verify_sig.rs b/update-format-crau/src/verify_sig.rs index fcfaff2..b2d2e53 100644 --- a/update-format-crau/src/verify_sig.rs +++ b/update-format-crau/src/verify_sig.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, Result, bail}; +use anyhow::{Context, Result, anyhow, bail}; use rsa::{RsaPrivateKey, RsaPublicKey}; use rsa::pkcs1::{DecodeRsaPrivateKey, DecodeRsaPublicKey}; use rsa::pkcs8::{DecodePrivateKey, DecodePublicKey}; @@ -40,7 +40,12 @@ pub fn verify_rsa_pkcs_buf(databuf: &[u8], signature: &[u8], public_key: RsaPubl let verifying_key = pkcs1v15::VerifyingKey::::new(public_key); - verifying_key.verify(databuf, &pkcs1v15::Signature::try_from(signature).unwrap()).context(format!("failed to verify signature ({:?})", signature)) + verifying_key + .verify( + databuf, + &pkcs1v15::Signature::try_from(signature).context(anyhow!("unable to convert signature into pkcs1v15::Signature"))?, + ) + .context(format!("failed to verify signature ({:?})", signature)) } // Takes a data buffer, signature and a public key, to verify the data @@ -52,11 +57,16 @@ pub fn verify_rsa_pkcs_buf(databuf: &[u8], signature: &[u8], public_key: RsaPubl pub fn verify_rsa_pkcs_prehash(digestbuf: &[u8], signature: &[u8], public_key: RsaPublicKey) -> Result<()> { let verifying_key = pkcs1v15::VerifyingKey::::new(public_key); - verifying_key.verify_prehash(digestbuf, &pkcs1v15::Signature::try_from(signature).unwrap()).context(format!("failed to verify_prehash signature ({:?})", signature)) + verifying_key + .verify_prehash( + digestbuf, + &pkcs1v15::Signature::try_from(signature).context(anyhow!("unable to convert signature into pkcs1v15::Signature"))?, + ) + .context(format!("failed to verify_prehash signature ({:?})", signature)) } pub fn get_private_key_pkcs_pem(private_key_path: &str, key_type: KeyType) -> Result { - let private_key_buf = fs::read_to_string(private_key_path).unwrap(); + let private_key_buf = fs::read_to_string(private_key_path).context(format!("failed to read private key from path {:?}", private_key_path))?; let out_key = match key_type { KeyType::KeyTypePkcs1 => RsaPrivateKey::from_pkcs1_pem(private_key_buf.as_str()).or_else(|error| { bail!("failed to parse PKCS1 PEM message: {:?}", error); @@ -73,7 +83,7 @@ pub fn get_private_key_pkcs_pem(private_key_path: &str, key_type: KeyType) -> Re } pub fn get_public_key_pkcs_pem(public_key_path: &str, key_type: KeyType) -> Result { - let public_key_buf = fs::read_to_string(public_key_path).unwrap(); + let public_key_buf = fs::read_to_string(public_key_path).context(format!("failed to read public key from path {:?}", public_key_path))?; let out_key = match key_type { KeyType::KeyTypePkcs1 => RsaPublicKey::from_pkcs1_pem(public_key_buf.as_str()).or_else(|error| { bail!("failed to parse PKCS1 PEM message: {:?}", error);