Skip to content

[WS-1] Web: js_sys::eval(format!()) for pinned-message scroll uses band-aid sanitization #425

@intendednull

Description

@intendednull

From #413.

Sev: low (defense-in-depth). Category: dangerous-eval / sanitization-band-aid. Obvious?: no.

crates/web/src/app.rs:1178-1181:

let on_pinned_jump = Callback::new(move |msg_id: String| {
    js_sys::eval(&format!(
        "document.getElementById('msg-{}')?.scrollIntoView({{behavior:'smooth',block:'center'}})",
        msg_id.replace('\'', "")
    )).ok();
    write.ui.set_show_pinned.set(false);
});

Sanitization strips single-quote only. Double-quote, backslash, newline, brackets, parens, etc. unstripped.

msg_id originates state::DisplayMessage::id: String (crates/state/src/types.rs and crates/client/src/state.rs:94) which is an EventHash::to_string() hex digest in current code. Reachable risk thus low — but:

  • band-aid sanitization rots; future code may put non-hex into id
  • pattern invites copy-paste into other call sites
  • safer DOM API exists

Fix: Replace with safe DOM API:

if let Some(elem) = web_sys::window()
    .and_then(|w| w.document())
    .and_then(|d| d.get_element_by_id(&format!("msg-{msg_id}")))
{
    let opts = web_sys::ScrollIntoViewOptions::new();
    opts.set_behavior(web_sys::ScrollBehavior::Smooth);
    opts.set_block(web_sys::ScrollLogicalPosition::Center);
    elem.scroll_into_view_with_scroll_into_view_options(&opts);
}

Other js_sys::eval sites (app.rs:23,50,71,1086,1090,1105, mobile_shell.rs:363, main.rs:17) use static JS strings — no injection surface.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions