[Bug] mtd-cfi erase-region parser has an off-by-one out-of-bounds write when sect_count reaches CFI_FLASH_SECT_MAX

[XueDugu]

RT-Thread Version

master (verified on commit 2b58dec87b584aa7ded6e8c736498716f8d29cd0)

Hardware Type/Architectures

systems using the new common MTD CFI NOR driver

Develop Toolchain

GCC

Describe the bug

Affected Components

Field	Detail
File	components/drivers/mtd/mtd-cfi.c
Function	CFI probe / erase-region parsing logic in cfi_flash_init()
Struct	struct cfi_flash_device
Fixed arrays	sect[CFI_FLASH_SECT_MAX], protect[CFI_FLASH_SECT_MAX]

---

Vulnerability Details

1. Fixed-capacity arrays indexed with an off-by-one check

The driver stores discovered flash sectors into two fixed-size arrays:

struct cfi_flash_device
{
    struct rt_mtd_nor_device parent;
    struct rt_mutex rw_lock;

    rt_ubase_t sect[CFI_FLASH_SECT_MAX];
    rt_ubase_t protect[CFI_FLASH_SECT_MAX];
    rt_size_t sect_count;
    ...
};

In the erase-region parser, the driver iterates over CFI-reported erase regions and appends entries into sect[] and protect[]. The bound check is:

if (sect_count > RT_ARRAY_SIZE(fdev->sect))
{
    LOG_E("Too many %d (> %d) sector found",
           sect_count, RT_ARRAY_SIZE(fdev->sect));
    break;
}

This condition should use >=, not >. As written, when sect_count == RT_ARRAY_SIZE(fdev->sect), the check still passes and the code performs:

fdev->sect[sect_count] = sect;
fdev->protect[sect_count] = ...;

which writes one element past the end of both arrays.

---

2. Overwrite driven by attacker-controlled CFI geometry

The number of iterations is derived from flash-reported CFI erase-region information:

value = rt_le32_to_cpu(get_unaligned(&query->erase_region_info[i]));

erase_region_count = (value & 0xffff) + 1;
value >>= 16;
erase_region_size = (value & 0xffff) ? ((value & 0xffff) * 256) : 128;

Then:

for (int j = 0; j < erase_region_count; ++j)
{
    ...
    fdev->sect[sect_count] = sect;
    ...
    fdev->protect[sect_count] = ...;
    ++sect_count;
}

A malicious, fuzzed, or emulated CFI NOR device can report enough erase-region data to drive sect_count up to exactly CFI_FLASH_SECT_MAX and trigger the off-by-one out-of-bounds write.

---

3. Concrete Vulnerable Code Path

for (int j = 0; j < erase_region_count; ++j)
{
    if (sect - fdev->sect[0] >= fdev->size)
    {
        break;
    }

    if (sect_count > RT_ARRAY_SIZE(fdev->sect))   // ← should be >=
    {
        LOG_E("Too many %d (> %d) sector found",
               sect_count, RT_ARRAY_SIZE(fdev->sect));
        break;
    }

    fdev->sect[sect_count] = sect;                // ← OOB write when sect_count == CFI_FLASH_SECT_MAX
    sect += (erase_region_size * size_ratio);

    switch (fdev->vendor)
    {
    case CFI_CMDSET_INTEL_PROG_REGIONS:
    case CFI_CMDSET_INTEL_EXTENDED:
    case CFI_CMDSET_INTEL_STANDARD:
        cfi_flash_write_cmd(fdev, sect_count, 0, FLASH_CMD_READ_ID);
        fdev->protect[sect_count] = cfi_flash_isset(fdev,  // ← OOB write
                sect_count, FLASH_OFFSET_PROTECT, FLASH_STATUS_PROTECT);
        cfi_flash_write_cmd(fdev, sect_count, 0, FLASH_CMD_RESET);
        break;

    case CFI_CMDSET_AMD_EXTENDED:
    case CFI_CMDSET_AMD_STANDARD:
    default:
        fdev->protect[sect_count] = RT_NULL;               // ← OOB write
    }

    ++sect_count;
}

At the point sect_count == CFI_FLASH_SECT_MAX, both fdev->sect[sect_count] and fdev->protect[sect_count] are out-of-bounds accesses.

---

Trigger Condition

The bug is reached during normal CFI probe/initialization of a NOR flash device. No local code modification is required. A malicious or emulated flash device only needs to provide CFI data with enough erase-region entries/counts to drive sect_count to exactly CFI_FLASH_SECT_MAX.

---

Proof of Concept

A PoC can be implemented with a CFI NOR emulator, a fuzzing harness that emulates the CFI query table, or a hardware test setup capable of returning crafted CFI query responses.

PoC Shape

Construct CFI erase-region information such that:

• The parser continues adding sectors until sect_count == 512
• At least one more sector append is attempted

At that point, the code will still pass the > bound check and perform:

fdev->sect[512] = ...
fdev->protect[512] = ...

even though both arrays are sized for indices 0..511.

Minimal Reproduction Steps

1. Build RT-Thread with the new common MTD CFI NOR driver enabled.
2. Provide a CFI-compatible flash device or emulator that reports enough erase regions/sectors to reach sect_count == 512.
3. Trigger flash probe/initialization.
4. Observe an out-of-bounds write when the parser appends the 513th entry.

Expected Result

The current code writes one element past the end of fdev->sect[] and fdev->protect[], potentially corrupting adjacent fields in struct cfi_flash_device or nearby memory depending on layout and build configuration.

---

Impact

The immediate impact is memory corruption during flash probe. Because this occurs while parsing externally supplied CFI geometry data, the bug may lead to:

• crash during probe
• corrupted flash metadata
• undefined behavior in later MTD operations
• possible corruption of adjacent driver state

Code execution has not been demonstrated.

---

Upstream / Downstream Impact

Upstream

Verified on current master in the new common MTD CFI NOR driver.

Downstream

Any downstream product based on current master that enables the common MTD CFI NOR driver and probes untrusted, emulated, or malformed CFI flash devices may be affected.

---

Suggested Fix

Change the bounds check to use >= to reject writes once sect_count reaches array capacity:

if (sect_count >= RT_ARRAY_SIZE(fdev->sect))
{
    LOG_E("Too many %d (>= %d) sector found",
           sect_count, RT_ARRAY_SIZE(fdev->sect));
    break;
}

Additionally consider:

• Validating the total accumulated number of sectors before the inner loop writes.
• Validating erase-region count values more defensively.
• Failing probe hard instead of silently truncating partially parsed geometry.

Kindly let me know if you intend to request a CVE ID upon confirmation of the vulnerability.

Other additional context

No response