code-423n4 / 2022-01-insure-findings

2 stars 0 forks source link

Ordering importance in a struct #334

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

Kumpirmafyas

Vulnerability details

Impact

The order of the "struct Template" in the Factory.sol contract is as follows: 1-bool isOpen 2-bool approval 3-bool allowDuplicate https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Factory.sol#L44-L48

The struct above is used in functions as value, in the "key=>value" part in this mapping. https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Factory.sol#L49

When using "template" mapping in this function, it is not done in the defined order, Detail:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Factory.sol#L101-L103

The problem here is; The order in which Structs are used in a Function is not. Problem ; The order of the structs in the "key => value" mapping definition affects the function. Sequencing is important in struct definition in mappings.

Recommended Mitigation Steps

The order in the struct = the order in the mapping = the order in the function must be the same.

Here ; Sorting in Mapping with Struct is a mandatory condition, while sorting in a function is within the scope of clean code.

oishun1112 commented 2 years ago

I see where we should fix. But, I don't see why this can be a High risk.

0xean commented 2 years ago

Issue is unclear as to why the ordering of assignment here is critical to the functionality on the code. Lowering to non-critical due to lack on information as this seems to be a code clarity issue.