Lack of limiting the length of `prefix` in function `check_prefix_length_isLegal`.

zlsecure3 commented 1 year ago

subject

Lack of limiting the length of prefix in function check_prefix_length_isLegal.

description

In function check_prefix_length_isLegal, the code only ensure that the length of prefix is greater than shortest_length, but not less than longest_length.

if (string::length(&prefix) > shortest_length) {
    return true
} else{
    return false
}

recommendation

Suggest that modifying the function as following:

public fun check_prefix_length_isLegal(collection: string::String,prefix: string::String) : bool acquires NameServicesFeeManager{
    assert!(exists<NameServicesFeeManager>(@SNS_address),ERROR);
    let name_service_extension = borrow_global_mut<NameServicesFeeManager>(@SNS_address);

    let shortest_length_table = &mut name_service_extension.domain_to_shortestlength;
    assert!(table::contains(&shortest_length_table, collection), NOT_EXIST);
    let shortest_length = *table::borrow(shortest_length_table,collection);

    let longest_length_table = &mut name_service_extension.domain_to_longestlength;
    assert!(table::contains(&longest_length_table, collection), NOT_EXIST);
    let longest_length = *table::borrow(longest_length_table,collection);

    if (string::length(&prefix) > shortest_length && string::length(&prefix) < longest_length) {
        return true
    } else{
        return false
    }
}

locations

code/sources/StarNameService.move#L108-118

severity

Medium

damage

exploitability

grading (edit)

submission_id:1466827695

review_type:GRADING

result: TBD-yes,no

rating: TBD-123

comment: TBD-Rejected,Accepted by Secure3.

severity: TBD-Critical,Medium,Low,Informational

category:

description:

zlsecure3 commented 1 year ago

client feedback (manual copy)

submission_id:1466827695

review_type:CLIENT_FEEDBACK

result: TBD-yes,no

severity: TBD-Critical,Medium,Low,Informational

comment:

zlsecure3 commented 1 year ago

client feedback decision(edit)

submission_id:1466827695

review_type:CLIENT_FEEDBACK_DECISION

result: TBD-yes,no,yes-honored,no-honored

severity: TBD-Critical,Medium,Low,Informational

comment:

zlsecure3 / review_star