diff --git a/src/actions.rs b/src/actions.rs index 3e0c039..8bd1031 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -1,5 +1,5 @@ use crate::context::SproutContext; -use anyhow::{Result, bail}; +use anyhow::{Context, Result, bail}; use serde::{Deserialize, Serialize}; use std::rc::Rc; @@ -50,7 +50,10 @@ pub fn execute(context: Rc, name: impl AsRef) -> Result<()> bail!("unknown action '{}'", name.as_ref()); }; // Finalize the context and freeze it. - let context = context.finalize().freeze(); + let context = context + .finalize() + .context("unable to finalize context")? + .freeze(); // Execute the action. if let Some(chainload) = &action.chainload { @@ -61,6 +64,7 @@ pub fn execute(context: Rc, name: impl AsRef) -> Result<()> return Ok(()); } else if let Some(edera) = &action.edera { edera::edera(context.clone(), edera)?; + return Ok(()); } #[cfg(feature = "splash")] diff --git a/src/actions/splash.rs b/src/actions/splash.rs index a51903b..991aecf 100644 --- a/src/actions/splash.rs +++ b/src/actions/splash.rs @@ -52,6 +52,11 @@ fn fit_to_frame(image: &DynamicImage, frame: Rect) -> Rect { height: image.height(), }; + // Handle the case where the image is zero-sized. + if input.height == 0 || input.width == 0 { + return input; + } + // Calculate the ratio of the image dimensions. let input_ratio = input.width as f32 / input.height as f32; @@ -66,6 +71,11 @@ fn fit_to_frame(image: &DynamicImage, frame: Rect) -> Rect { height: frame.height, }; + // Handle the case where the output is zero-sized. + if output.height == 0 || output.width == 0 { + return output; + } + if input_ratio < frame_ratio { output.width = (frame.height as f32 * input_ratio).floor() as u32; output.height = frame.height; @@ -110,7 +120,8 @@ fn draw(image: DynamicImage) -> Result<()> { let image = resize_to_fit(&image, fit); // Create a framebuffer to draw the image on. - let mut framebuffer = Framebuffer::new(width, height); + let mut framebuffer = + Framebuffer::new(width, height).context("unable to create framebuffer")?; // Iterate over the pixels in the image and put them on the framebuffer. for (x, y, pixel) in image.enumerate_pixels() { diff --git a/src/context.rs b/src/context.rs index 8755e83..e299325 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,11 +1,15 @@ use crate::actions::ActionDeclaration; use crate::options::SproutOptions; -use anyhow::Result; use anyhow::anyhow; +use anyhow::{Result, bail}; +use std::cmp::Reverse; use std::collections::{BTreeMap, BTreeSet}; use std::rc::Rc; use uefi::proto::device_path::DevicePath; +/// The maximum number of iterations that can be performed in [SproutContext::finalize]. +const CONTEXT_FINALIZE_ITERATION_LIMIT: usize = 100; + /// Declares a root context for Sprout. /// This contains data that needs to be shared across Sprout. #[derive(Default)] @@ -151,11 +155,20 @@ impl SproutContext { /// Finalizes a context by producing a context with no parent that contains all the values /// of all parent contexts merged. This makes it possible to ensure [SproutContext] has no /// inheritance with other [SproutContext]s. It will still contain a [RootContext] however. - pub fn finalize(&self) -> SproutContext { + pub fn finalize(&self) -> Result { // Collect all the values from the context and its parents. let mut current_values = self.all_values(); + // To ensure that there is no possible infinite loop, we need to check + // the number of iterations. If it exceeds 100, we bail. + let mut iterations: usize = 0; loop { + iterations += 1; + + if iterations > CONTEXT_FINALIZE_ITERATION_LIMIT { + bail!("infinite loop detected in context finalization"); + } + let mut did_change = false; let mut values = BTreeMap::new(); for (key, value) in ¤t_values { @@ -176,11 +189,11 @@ impl SproutContext { } // Produce the final context. - Self { + Ok(Self { root: self.root.clone(), parent: None, values: current_values, - } + }) } /// Stamps the `text` value with the specified `values` map. The returned value indicates @@ -188,7 +201,25 @@ impl SproutContext { fn stamp_values(values: &BTreeMap, text: impl AsRef) -> (bool, String) { let mut result = text.as_ref().to_string(); let mut did_change = false; - for (key, value) in values { + + // Sort the keys by length. This is to ensure that we stamp the longest keys first. + // If we did not do this, "$abc" could be stamped by "$a" into an invalid result. + let mut keys = values.keys().collect::>(); + + // Sort by key length, reversed. This results in the longest keys appearing first. + keys.sort_by_key(|key| Reverse(key.len())); + + for key in keys { + // Empty keys are not supported. + if key.is_empty() { + continue; + } + + // We can fetch the value from the map. It is verifiable that the key exists. + let Some(value) = values.get(key) else { + unreachable!("keys iterated over is collected on a map that cannot be modified"); + }; + let next_result = result.replace(&format!("${key}"), value); if result != next_result { did_change = true; diff --git a/src/extractors/filesystem_device_match.rs b/src/extractors/filesystem_device_match.rs index 59ebf76..54a91ec 100644 --- a/src/extractors/filesystem_device_match.rs +++ b/src/extractors/filesystem_device_match.rs @@ -81,7 +81,7 @@ pub fn extract( } else { // We should still handle other errors gracefully. Err(error).context("unable to open filesystem partition info")?; - None + unreachable!() } } } diff --git a/src/generators/bls.rs b/src/generators/bls.rs index cd26391..b7c6b6c 100644 --- a/src/generators/bls.rs +++ b/src/generators/bls.rs @@ -6,7 +6,6 @@ use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; use std::rc::Rc; use std::str::FromStr; -use uefi::CString16; use uefi::fs::{FileSystem, Path}; use uefi::proto::device_path::text::{AllowShortcuts, DisplayOnly}; use uefi::proto::media::fs::SimpleFileSystem; @@ -89,10 +88,9 @@ pub fn generate(context: Rc, bls: &BlsConfiguration) -> Result Option { self.initrd .clone() - .map(|path| path.replace("/", "\\").trim_start_matches("\\").to_string()) + .map(|path| path.replace('/', "\\").trim_start_matches('\\').to_string()) } /// Fetches the options to pass to the kernel, if any. diff --git a/src/main.rs b/src/main.rs index f3b6474..9a5d5bc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -107,7 +107,7 @@ fn main() -> Result<()> { context.insert(&extracted); let context = context.freeze(); - // Execute the late phase. + // Execute the startup phase. phase(context.clone(), &config.phases.startup).context("unable to execute startup phase")?; let mut entries = Vec::new(); @@ -143,7 +143,10 @@ fn main() -> Result<()> { // Insert the values from the entry configuration into the // sprout context to use with the entry itself. context.insert(&entry.declaration().values); - let context = context.finalize().freeze(); + let context = context + .finalize() + .context("unable to finalize context")? + .freeze(); // Provide the new context to the bootable entry. entry.swap_context(context); // Restamp the title with any values. diff --git a/src/options/parser.rs b/src/options/parser.rs index 23b50ad..d6cee40 100644 --- a/src/options/parser.rs +++ b/src/options/parser.rs @@ -72,11 +72,7 @@ pub trait OptionsRepresentable { let mut value = None; // Check if the option is of the form --abc=123 - if option.contains("=") { - let Some((part_key, part_value)) = option.split_once("=") else { - bail!("invalid option: {option}"); - }; - + if let Some((part_key, part_value)) = option.split_once('=') { let part_key = part_key.to_string(); let part_value = part_value.to_string(); option = part_key; @@ -131,7 +127,7 @@ pub trait OptionsRepresentable { ); } // Exit because the help has been displayed. - std::process::exit(1); + std::process::exit(0); } // Insert the option and the value into the map. diff --git a/src/utils.rs b/src/utils.rs index 24edafd..1179df9 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -27,6 +27,12 @@ pub fn text_to_device_path(path: &str) -> Result { .context("unable to convert text to device path") } +/// Checks if a [CString16] contains a char `c`. +/// We need to call to_string() because CString16 doesn't support `contains` with a char. +fn cstring16_contains_char(string: &CString16, c: char) -> bool { + string.to_string().contains(c) +} + /// Grabs the root part of the `path`. /// For example, given "PciRoot(0x0)/Pci(0x4,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)/HD(1,MBR,0xBE1AFDFA,0x3F,0xFBFC1)/\EFI\BOOT\BOOTX64.efi" /// it will give "PciRoot(0x0)/Pci(0x4,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)/HD(1,MBR,0xBE1AFDFA,0x3F,0xFBFC1)" @@ -37,7 +43,7 @@ pub fn device_path_root(path: &DevicePath) -> Result { let item = item.to_string(DisplayOnly(false), AllowShortcuts(false)); if item .as_ref() - .map(|item| item.to_string().contains("(")) + .map(|item| cstring16_contains_char(item, '(')) .unwrap_or(false) { Some(item.unwrap_or_default()) @@ -62,7 +68,7 @@ pub fn device_path_subpath(path: &DevicePath) -> Result { let item = item.to_string(DisplayOnly(false), AllowShortcuts(false)); if item .as_ref() - .map(|item| item.to_string().contains("(")) + .map(|item| cstring16_contains_char(item, '(')) .unwrap_or(false) { None @@ -104,11 +110,11 @@ pub fn resolve_path(default_root_path: &DevicePath, input: &str) -> Result Self { - Framebuffer { + pub fn new(width: usize, height: usize) -> Result { + // Verify that the size is valid during multiplication. + let size = width + .checked_mul(height) + .context("framebuffer size overflow")?; + + // Initialize the pixel buffer with black pixels, with the verified size. + let pixels = vec![BltPixel::new(0, 0, 0); size]; + + Ok(Framebuffer { width, height, - pixels: vec![BltPixel::new(0, 0, 0); width * height], - } + pixels, + }) } /// Mutably acquires a pixel of the framebuffer at the specified `x` and `y` coordinate. pub fn pixel(&mut self, x: usize, y: usize) -> Option<&mut BltPixel> { - self.pixels.get_mut(y * self.width + x) + // Verify that the coordinates are within the bounds of the framebuffer. + if x >= self.width || y >= self.height { + return None; + } + + // Calculate the index of the pixel safely, returning None if it overflows. + let index = y.checked_mul(self.width)?.checked_add(x)?; + // Return the pixel at the index. If the index is out of bounds, this will return None. + self.pixels.get_mut(index) } /// Blit the framebuffer to the specified `gop` [GraphicsOutput]. diff --git a/src/utils/media_loader.rs b/src/utils/media_loader.rs index d8840ac..f3f4954 100644 --- a/src/utils/media_loader.rs +++ b/src/utils/media_loader.rs @@ -97,7 +97,7 @@ impl MediaLoaderHandle { } /// Creates a new device path for the media loader based on a vendor `guid`. - fn device_path(guid: Guid) -> Box { + fn device_path(guid: Guid) -> Result> { // The buffer for the device path. let mut path = Vec::new(); // Build a device path for the media loader with a vendor-specific guid. @@ -106,18 +106,18 @@ impl MediaLoaderHandle { vendor_guid: guid, vendor_defined_data: &[], }) - .unwrap() // We know that the device path is valid, so we can unwrap. + .context("unable to produce device path")? .finalize() - .unwrap(); // We know that the device path is valid, so we can unwrap. + .context("unable to produce device path")?; // Convert the device path to a boxed device path. // This is safer than dealing with a pooled device path. - path.to_boxed() + Ok(path.to_boxed()) } /// Checks if the media loader is already registered with the UEFI stack. fn already_registered(guid: Guid) -> Result { // Acquire the device path for the media loader. - let path = Self::device_path(guid); + let path = Self::device_path(guid)?; let mut existing_path = path.as_ref(); @@ -142,7 +142,7 @@ impl MediaLoaderHandle { /// to load the data from. pub fn register(guid: Guid, data: Box<[u8]>) -> Result { // Acquire the vendor device path for the media loader. - let path = Self::device_path(guid); + let path = Self::device_path(guid)?; // Check if the media loader is already registered. // If it is, we can't register it again safely.