lowRISC / lowRISC/opentitan

[otp_ctrl] Bug in `dif_otp_ctrl_read_blocking` causes out of range check to not execute when it should

Open
#27,829 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

### Description

The function `dif_otp_ctrl_read_blocking` in `sw/device/lib/dif/dif_otp_ctrl.c` reads from the memory-mapped software config OTP partitions. It takes a parameter `size_t len`, which is documented as "The number of words to read".

The last few lines of `dif_otp_ctrl_read_blocking` look like this:

```c
if (address + len >= kPartitions[partition].len) {
return kDifOutOfRange;
}

uint32_t reg_offset = OTP_CTRL_SW_CFG_WINDOW_REG_OFFSET +
kPartitions[partition].start_addr + address;
mmio_region_memcpy_from_mmio32(otp->base_addr, reg_offset, buf,
len * sizeof(uint32_t));
return kDifOk;
```

The out of range check here is broken; `kPartitions[partition].len` represents a quantity of bytes, per the documentation for the `uint32_t len` field in `partition_info_t`: "The length of this partition, in bytes, including the digest". `address + len` should instead be `address + len * sizeof(uint32_t)` so that all units are converted to bytes before comparison.

The resulting behavior here is that the caller can read beyond the boundary of a memory-mapped partition, when they should not be able to. "Garbage" data is read into the buffer; no type of fault occurs.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in sw/device/lib/dif/dif_otp_ctrl.c at dif_otp_ctrl_read_blocking and inspect the partition length units alongside the read offset and copy length. Confirm that boundary checks reject reads beyond a partition while still allowing valid reads, and verify the behavior with the relevant DIF tests if available.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
embedded-iot, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.