[Low] Potential panic in offchain worker due to unwrap

[sebastianmontero]

[Low] Potential panic in offchain worker due to unwrap

Summary

There is a potential panic in the offchain_worker of the bitcoin-vaults pallet. This is due to missing input validation.

Issue details

xpubs are stored as BoundedVec<u8, T::XPubLen>:

#[pallet::storage]
#[pallet::getter(fn xpubs)]
pub(super) type Xpubs<T: Config> = StorageMap<
    _,
    Identity,
    [u8; 32], //that's the blake 2 hash result
    BoundedVec<u8, T::XPubLen>,
    OptionQuery,
>;

xpubs are inserted when calling the set_xpub extrinsic. Callers provide a BoundedVec of u8 elements. The code does not perform any validation on the xpubs when inserting them into the storage map:

#[pallet::call_index(1)]
#[pallet::weight(Weight::from_parts(10_000,0) + T::DbWeight::get().writes(2))]
pub fn set_xpub(origin: OriginFor<T>, xpub: BoundedVec<u8, T::XPubLen>) -> DispatchResult {
    // Check that the extrinsic was signed and get the signer.
    let who = ensure_signed(origin.clone())?;
    ensure!(xpub.len() > 0, Error::<T>::NoneValue);
    ensure!(!<XpubsByOwner<T>>::contains_key(who.clone()), Error::<T>::UserAlreadyHasXpub);
    let manual_hash = xpub.clone().using_encoded(blake2_256);
    // Assert if the input xpub is free to take (or if the user owns it)
    match Self::get_xpub_status(who.clone(), manual_hash.clone()) {
        // ...
        XpubStatus::Free => {
// --> `xpub` inserted without UTF8 validity checks expected by function later on (see comments below) 
            <Xpubs<T>>::insert(manual_hash, xpub.clone());

Later on, as part of the offchain_worker processing, generate_vault_json_body attempts to UTF8-decode an account's xpubs using the following code:

let mapped_xpubs: Vec<JsonValue> = xpubs
    .iter()
    .map(|xpub| {
// --> Panic if `xpub` not valid UTF8 encoding
        let xpub_field = JsonValue::String(str::from_utf8(xpub).unwrap().chars().collect());
        JsonValue::Object([("xpub".chars().collect(), xpub_field)].to_vec())
    })
    .collect();

This code will attempt to decode an xpub as UTF8 data, which might fail if malformed data was provided in a call to set_xpub before. Because unwrap is used, the code will panic if UTF8-decoding fails.

Risk

If attackers are able to insert their own malformed xpubs, they can crash the offchain worker, negatively impacting blockchain availability.

Mitigation

Validate the expected encoding when inserting the xpub or otherwise gracefully handle decoding errors.