Harden lock UI inputs and decoding and reduce idle clock updates

shell/ferese-lock/src/appearance.rs

@@ -140,7 +140,7 @@ fn account_picture(user: &str) -> Option<image::Handle> {
         candidates.push(std::path::PathBuf::from(home).join(".face"));
     }
     candidates.into_iter().find_map(|path| {
-        let pixels = cosmic::iced::advanced::graphics::image::load(&image::Handle::from_path(path)).ok()?;
+        let pixels = decode_avatar(&path).ok()?;
         let side = pixels.width().min(pixels.height());
         if side == 0 {
             return None;
@@ -169,6 +169,50 @@ fn account_picture(user: &str) -> Option<image::Handle> {
     })
 }
 
+fn decode_avatar(path: &std::path::Path) -> Result<::image::RgbaImage, String> {
+    use std::io::Read;
+    use std::os::unix::fs::OpenOptionsExt;
+
+    use ::image::ImageDecoder;
+
+    const ENCODED_LIMIT: u64 = 8 * 1024 * 1024;
+    const DECODED_LIMIT: u64 = 32 * 1024 * 1024;
+    let file = std::fs::OpenOptions::new()
+        .read(true)
+        .custom_flags(libc::O_NONBLOCK)
+        .open(path)
+        .map_err(|error| error.to_string())?;
+    if !file.metadata().map_err(|error| error.to_string())?.is_file() {
+        return Err("Avatar is not a regular file".into());
+    }
+
+    let mut bytes = Vec::new();
+    file.take(ENCODED_LIMIT + 1)
+        .read_to_end(&mut bytes)
+        .map_err(|error| error.to_string())?;
+    if bytes.len() as u64 > ENCODED_LIMIT {
+        return Err("Avatar exceeds the file limit".into());
+    }
+
+    let mut reader = ::image::ImageReader::new(std::io::Cursor::new(bytes))
+        .with_guessed_format()
+        .map_err(|error| error.to_string())?;
+    let mut limits = ::image::Limits::default();
+    limits.max_image_width = Some(4096);
+    limits.max_image_height = Some(4096);
+    limits.max_alloc = Some(DECODED_LIMIT);
+    reader.limits(limits);
+    let decoder = reader.into_decoder().map_err(|error| error.to_string())?;
+    let (width, height) = decoder.dimensions();
+    if width == 0 || height == 0 || u64::from(width) * u64::from(height) * 4 > DECODED_LIMIT {
+        return Err("Avatar exceeds the decode limit".into());
+    }
+
+    ::image::DynamicImage::from_decoder(decoder)
+        .map(|image| image.into_rgba8())
+        .map_err(|error| error.to_string())
+}
+
 pub fn load_wallpaper(path: &std::path::Path, blur: f32) -> Result<image::Handle, String> {
     use ::image::ImageDecoder;
     let mut reader = ::image::ImageReader::open(path)
@@ -201,6 +245,23 @@ pub fn load_wallpaper(path: &std::path::Path, blur: f32) -> Result<image::Handle
 #[cfg(test)]
 mod tests {
     use super::*;
+    #[test]
+    fn avatar_decoding_bounds_dimensions_and_encoded_size() {
+        let file = tempfile::NamedTempFile::new().unwrap();
+        ::image::RgbaImage::new(3, 2)
+            .save_with_format(file.path(), ::image::ImageFormat::Png)
+            .unwrap();
+        assert_eq!(decode_avatar(file.path()).unwrap().dimensions(), (3, 2));
+
+        ::image::RgbaImage::new(4097, 1)
+            .save_with_format(file.path(), ::image::ImageFormat::Png)
+            .unwrap();
+        assert!(decode_avatar(file.path()).is_err());
+
+        file.as_file().set_len(8 * 1024 * 1024 + 1).unwrap();
+        assert!(decode_avatar(file.path()).unwrap_err().contains("file limit"));
+    }
+
     #[test]
     fn portal_wallpaper_loads_without_a_filename_extension() {
         let file = tempfile::NamedTempFile::new().unwrap();

shell/ferese-lock/src/main.rs

@@ -85,10 +85,11 @@ enum Message {
     ThemeChanged(Box<ferese_config::theme::Snapshot>),
     WallpaperLoaded(u64, Result<widget::image::Handle, String>),
     Event(Box<Event>),
-    Input(String),
+    Input(Zeroizing<String>),
     Submit,
     Authenticated(bool),
     Tick,
+    RetryReady,
 }
 
 // Never derive Debug for password-bearing messages.
@@ -203,7 +204,17 @@ impl cosmic::Application for Locker {
                 | Event::Keyboard(iced::keyboard::Event::ModifiersChanged(_)) => Some(Message::Event(Box::new(event))),
                 _ => None,
             }),
-            iced::time::every(Duration::from_secs(1)).map(|_| Message::Tick),
+            if self.appearance.show_clock || self.appearance.show_date {
+                Subscription::run(runtime::minute_ticks).map(|_| Message::Tick)
+            } else {
+                Subscription::none()
+            },
+            match self.state {
+                AuthState::Rejected(at) => {
+                    Subscription::run_with(at, runtime::retry_deadline).map(|_| Message::RetryReady)
+                }
+                _ => Subscription::none(),
+            },
         ])
     }
 
@@ -260,18 +271,18 @@ impl cosmic::Application for Locker {
                 let now = jiff::Zoned::now();
                 self.clock = now.strftime(self.appearance.clock_format()).to_string();
                 self.date = now.strftime("%A, %B %-d").to_string();
-
+            }
+            Message::RetryReady => {
                 if matches!(self.state, AuthState::Rejected(at) if at.elapsed() >= Duration::from_secs(2)) {
                     self.state = AuthState::Ready;
                 }
             }
-            Message::Input(mut value) => {
+            Message::Input(value) => {
                 if self.state.may_submit() && value.len() <= 4096 {
                     self.password.zeroize();
-                    *self.password = std::mem::take(&mut value);
+                    self.password = value;
                     self.status = "Press Enter to unlock";
                 }
-                value.zeroize();
             }
             Message::Submit => {
                 if self.preview {
@@ -390,31 +401,34 @@ impl Locker {
 
     fn screen_content(&self, size: iced::Size) -> Element<'_, Message> {
         let avatar_size = (size.height * 0.08).clamp(56., 88.);
-        let clock_size = (size.height * 0.165)
-            .min(size.width * if self.appearance.twelve_hour { 0.075 } else { 0.12 })
-            .clamp(40., 180.) as u16;
+        let clock_size = (size.height * 0.06)
+            .min(size.width * if self.appearance.twelve_hour { 0.055 } else { 0.085 })
+            .clamp(32., 72.) as u16;
         let margin = if size.width < 640. { 24. } else { 36. };
         let top = (size.height * 0.09).max(24.);
         let a = &self.appearance;
         let mut input = widget::text_input("Password", self.password.as_str())
             .password()
-            .padding([6, 12])
+            .padding([4, 10])
+            .size(14)
             .style(ferese_theme::controls::lock_input(a.radius, a.accent, a.panel))
             .font(a.font)
             .id(widget::Id::new("password"));
 
         if self.state.may_submit() {
-            input = input.on_input(Message::Input).on_submit(|mut value| {
-                value.zeroize();
-                Message::Submit
-            });
+            input = input
+                .on_input(|value| Message::Input(Zeroizing::new(value)))
+                .on_submit(|value| {
+                    drop(Zeroizing::new(value));
+                    Message::Submit
+                });
         }
 
-        let submit = widget::button::custom(widget::icon::from_name("go-next-symbolic").size(20))
+        let submit = widget::button::custom(widget::icon::from_name("go-next-symbolic").size(16))
             .class(theme::Button::Icon)
-            .padding(6)
+            .padding(4)
             .on_press_maybe(self.state.may_submit().then_some(Message::Submit));
-        input = input.trailing_icon(container(submit).padding([0, 6]).into());
+        input = input.trailing_icon(container(submit).padding([0, 4]).into());
         let avatar: Element<'_, Message> = if let Some(photo) = &a.avatar {
             image(photo.clone())
                 .width(avatar_size)
@@ -457,8 +471,8 @@ impl Locker {
         .spacing(12)
         .align_x(iced::Alignment::Center)
         .width(
-            (size.height * 0.31)
-                .clamp(240., 320.)
+            (size.height * 0.23)
+                .clamp(200., 240.)
                 .min((size.width - margin * 2.).max(160.)),
         );
         let mut clock = column([]).spacing(0).align_x(iced::Alignment::Center);
@@ -559,6 +573,6 @@ mod tests {
     }
     #[test]
     fn password_messages_are_redacted() {
-        assert!(!format!("{:?}", Message::Input("secret".into())).contains("secret"));
+        assert!(!format!("{:?}", Message::Input(Zeroizing::new("secret".into()))).contains("secret"));
     }
 }

shell/ferese-lock/src/runtime.rs

@@ -93,3 +93,39 @@ mod tests {
         assert!(!c.observe());
     }
 }
+
+fn until_next_minute(since_epoch: Duration) -> Duration {
+    Duration::from_secs(60) - Duration::new(since_epoch.as_secs() % 60, since_epoch.subsec_nanos())
+}
+
+pub fn minute_ticks() -> impl cosmic::iced::futures::Stream<Item = ()> {
+    cosmic::iced::futures::stream::unfold((), |()| async {
+        let now = std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap_or_default();
+        tokio::time::sleep(until_next_minute(now)).await;
+        Some(((), ()))
+    })
+}
+
+pub fn retry_deadline(at: &std::time::Instant) -> impl cosmic::iced::futures::Stream<Item = ()> + use<> {
+    let at = *at;
+    cosmic::iced::futures::stream::once(async move {
+        tokio::time::sleep(Duration::from_secs(2).saturating_sub(at.elapsed())).await;
+    })
+}
+
+#[cfg(test)]
+mod clock_tests {
+    use super::*;
+
+    #[test]
+    fn minute_deadline_uses_the_wall_clock_boundary() {
+        assert_eq!(until_next_minute(Duration::ZERO), Duration::from_secs(60));
+        assert_eq!(
+            until_next_minute(Duration::from_millis(119_999)),
+            Duration::from_millis(1)
+        );
+        assert_eq!(until_next_minute(Duration::from_secs(125)), Duration::from_secs(55));
+    }
+}