Repository: google/OpenSK
File: src/ctap/command.rs (Rust)

Code before:

        let client_data_hash = read_byte_string(ok_or_missing(param_map.get(&cbor_unsigned!(1)))?)?;

        let rp = PublicKeyCredentialRpEntity::try_from(ok_or_missing(
            param_map.get(&cbor_unsigned!(2)),
        )?)?;

        let user = PublicKeyCredentialUserEntity::try_from(ok_or_missing(
            param_map.get(&cbor_unsigned!(3)),
        )?)?;

        let cred_param_vec = read_array(ok_or_missing(param_map.get(&cbor_unsigned!(4)))?)?;
        let mut pub_key_cred_params = vec![];
        for cred_param_map_value in cred_param_vec {
            if let Ok(cred_param) = PublicKeyCredentialParameter::try_from(cred_param_map_value) {
                pub_key_cred_params.push(cred_param);
            }
        }

        let exclude_list = match param_map.get(&cbor_unsigned!(5)) {
            Some(entry) => {
                let exclude_list_vec = read_array(entry)?;
                let mut exclude_list = vec![];
                for exclude_list_value in exclude_list_vec {
                    if let Some(count) = MAX_CREDENTIAL_COUNT_IN_LIST {
                        if exclude_list.len() as u64 >= count {
                            break;
                        }
                    }
                    exclude_list.push(PublicKeyCredentialDescriptor::try_from(exclude_list_value)?);
                }
                Some(exclude_list)
            }
            None => None,
        };

        let extensions = param_map
            .get(&cbor_unsigned!(6))
            .map(Extensions::try_from)
            .transpose()?;

        let options = match param_map.get(&cbor_unsigned!(7)) {
            Some(entry) => MakeCredentialOptions::try_from(entry)?,
            None => MakeCredentialOptions {
                rk: false,
                uv: false,
            },
        };

        let pin_uv_auth_param = param_map
            .get(&cbor_unsigned!(8))

Review comment (bug):
`MAX_CREDENTIAL_COUNT_IN_LIST` should be a `usize` instead.

As far as I understand it, this parameter is local to this code, i.e. isn't used in any protocol, so a native size should be fine. Also, is there a use case where this would overflow an `u32`, to warrant using a `u64`? Unless there is more than 4GB of RAM, I don't see how that would ever happen.

Code after:
        let param_map = read_map(&cbor_value)?;

        let client_data_hash = read_byte_string(ok_or_missing(param_map.get(&cbor_unsigned!(1)))?)?;

        let rp = PublicKeyCredentialRpEntity::try_from(ok_or_missing(
            param_map.get(&cbor_unsigned!(2)),
        )?)?;

        let user = PublicKeyCredentialUserEntity::try_from(ok_or_missing(
            param_map.get(&cbor_unsigned!(3)),
        )?)?;

        let cred_param_vec = read_array(ok_or_missing(param_map.get(&cbor_unsigned!(4)))?)?;
        let pub_key_cred_params = cred_param_vec
            .iter()
            .map(PublicKeyCredentialParameter::try_from)
            .collect::<Result<Vec<PublicKeyCredentialParameter>, Ctap2StatusCode>>()?;

        let exclude_list = match param_map.get(&cbor_unsigned!(5)) {
            Some(entry) => {
                let exclude_list_vec = read_array(entry)?;
                let exclude_list = exclude_list_vec
                    .iter()
                    .take(MAX_CREDENTIAL_COUNT_IN_LIST.unwrap_or(exclude_list_vec.len()))
                    .map(PublicKeyCredentialDescriptor::try_from)
                    .collect::<Result<Vec<PublicKeyCredentialDescriptor>, Ctap2StatusCode>>()?;
                Some(exclude_list)
            }
            None => None,
        };

        let extensions = param_map
            .get(&cbor_unsigned!(6))
            .map(Extensions::try_from)
            .transpose()?;

        let options = match param_map.get(&cbor_unsigned!(7)) {
            Some(entry) => MakeCredentialOptions::try_from(entry)?,
            None => MakeCredentialOptions {
                rk: false,
                uv: false,
            },
        };

        let pin_uv_auth_param = param_map
            .get(&cbor_unsigned!(8))
            .map(read_byte_string)
            .transpose()?;

        let pin_uv_auth_protocol = param_map
            .get(&cbor_unsigned!(9))