Fix buffer size mismatch (#114)

[P-rth]

closes #114

When a smaller buffer size is used, the non-overwritten bytes from the old buffer remain in memory, causing a mismatch/garbage data.

fix:
Added a 0 initialization to the buffer before copying the new data over to ensure no old bytes are left behind.

Summary by Sourcery

Ensure badge configuration buffers are cleared before copying new BLE device name and splash screen bitmap data to prevent stale bytes when shorter payloads are written.

Bug Fixes:

• Zero-initialize BLE device name and splash screen bitmap buffers before copying incoming data to avoid leftover data when new content is shorter than the previous one.

Enhancements:

• Zero out splash screen bitmap buffer during fallback configuration to maintain consistent initialization and prevent residual data.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR ensures fixed-size configuration buffers are fully cleared before writing potentially smaller payloads into them, preventing leftover bytes from prior content from leaking into current values.

Class diagram for badge configuration buffers and writers

classDiagram
    class badge_cfg_struct {
        uint8_t ble_devname[]
        uint8_t splash_bm_bits[]
        uint16_t splash_bm_w
        uint16_t splash_bm_h
        uint8_t splash_speedI
        uint8_t splash_speedT
    }

    class ngctrl_c {
        +uint8_t power_setting(uint8_t* val, uint16_t len)
        +void cfg_ble_devname(uint8_t* name, uint16_t len)
        +uint8_t flash_splash_screen(uint8_t* val, uint16_t len)
    }

    class config_c {
        +void cfg_fallback()
    }

    ngctrl_c --> badge_cfg_struct : writes_ble_devname
    ngctrl_c --> badge_cfg_struct : writes_splash_bm_fields
    config_c --> badge_cfg_struct : initializes_fallback_splash

    class BufferWriteBehavior {
        +clear_ble_devname_before_write()
        +clear_splash_bm_bits_before_write()
    }

    BufferWriteBehavior <|.. ngctrl_c
    BufferWriteBehavior <|.. config_c

Loading

Flow diagram for clearing and updating splash screen buffer

flowchart TD
    A[flash_splash_screen called with val and len] --> B{len < 3?}
    B -- Yes --> C[Return error -4]
    B -- No --> D[Compute sz, w, h]
    D --> E[tmos_memset badge_cfg.splash_bm_bits to 0 with sizeof badge_cfg.splash_bm_bits]
    E --> F[tmos_memcpy from val offset 3 into badge_cfg.splash_bm_bits with size sz]
    F --> G[Set badge_cfg.splash_bm_w to w]
    G --> H[Set badge_cfg.splash_bm_h to h]
    H --> I[Return success]

    subgraph Fallback_config_path
        J[cfg_fallback called] --> K[Compute splash_size as ALIGN_1BYTE splash.w times splash.h]
        K --> L[memset badge_cfg.splash_bm_bits to 0 with sizeof badge_cfg.splash_bm_bits]
        L --> M[memcpy splash.bits into badge_cfg.splash_bm_bits with size splash_size]
        M --> N[Set badge_cfg.splash_bm_w to splash.w]
        N --> O[Set badge_cfg.splash_bm_h to splash.h]
    end

Loading

Flow diagram for clearing and updating BLE device name buffer

flowchart TD
    A[cfg_ble_devname called with name and len] --> B[tmos_memset badge_cfg.ble_devname to 0 with sizeof badge_cfg.ble_devname]
    B --> C[tmos_memcpy name into badge_cfg.ble_devname with size len]
    C --> D[Return]

Loading

File-Level Changes

Change	Details	Files
Zero-initialize BLE device name buffer before copying new name data to avoid leftover bytes when new name is shorter.	Added a memset wrapper call to clear the BLE device name configuration buffer to all zeros before each update Preserved existing copy logic that writes the provided name into the configuration buffer using the provided length	src/ngctrl.c
Zero-initialize splash screen bitmap buffer before copying new bitmap data, both for runtime updates and fallback configuration.	Cleared the splash bitmap configuration buffer to zeros before copying new splash bitmap bits in the BLE-controlled flash_splash_screen path Cleared the same splash bitmap buffer to zeros in the cfg_fallback path before copying default splash bitmap bits Left existing width/height metadata assignments unchanged after the bitmap copy	src/ngctrl.c src/config.c

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#114	Ensure that when configuring the BLE device name, any previous content beyond the new name length is cleared so that shorter names do not leave stale bytes in the buffer.	✅
#114	Ensure that when configuring the splash screen bitmap (including via Bluetooth and via the fallback configuration), any previous content beyond the new image size is cleared so that smaller images do not leave stale bytes in the buffer.	✅

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• In cfg_ble_devname and flash_splash_screen, consider guarding the tmos_memcpy calls with min(len, sizeof(...)) (or an explicit length check) so that you cannot overflow the destination buffers if a larger-than-expected length is passed in.
• In cfg_fallback you use memset/memcpy while in ngctrl.c you use tmos_memset/tmos_memcpy; it would be good to standardize on one set of APIs here for consistency and to avoid subtle behavioral differences on this platform.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `cfg_ble_devname` and `flash_splash_screen`, consider guarding the `tmos_memcpy` calls with `min(len, sizeof(...))` (or an explicit length check) so that you cannot overflow the destination buffers if a larger-than-expected length is passed in.
- In `cfg_fallback` you use `memset`/`memcpy` while in `ngctrl.c` you use `tmos_memset`/`tmos_memcpy`; it would be good to standardize on one set of APIs here for consistency and to avoid subtle behavioral differences on this platform.

## Individual Comments

### Comment 1
<location path="src/ngctrl.c" line_range="60-61" />
<code_context>

 static void cfg_ble_devname(uint8_t *name, uint16_t len)
 {
+	tmos_memset(badge_cfg.ble_devname, 0, sizeof(badge_cfg.ble_devname));
 	tmos_memcpy(badge_cfg.ble_devname, name, len);
 }

</code_context>
<issue_to_address>
**🚨 issue (security):** Guard `len` against `badge_cfg.ble_devname` capacity when writing to the buffer.

Since this function now always operates on the full `ble_devname` buffer, please clamp or validate `len` so `tmos_memcpy` cannot write past `badge_cfg.ble_devname` (e.g., `len = MIN(len, sizeof(badge_cfg.ble_devname))` or return an error if too large).
</issue_to_address>

### Comment 2
<location path="src/config.c" line_range="33-34" />
<code_context>
 	badge_cfg.splash_speedT = 30; // ms

 	int splash_size = ALIGN_1BYTE(splash.w) * splash.h;
+	memset(badge_cfg.splash_bm_bits, 0, sizeof(badge_cfg.splash_bm_bits));
 	memcpy(badge_cfg.splash_bm_bits, splash.bits, splash_size);
 	badge_cfg.splash_bm_w = splash.w;
 	badge_cfg.splash_bm_h = splash.h;
</code_context>
<issue_to_address>
**issue (bug_risk):** Validate that `splash_size` fits into `badge_cfg.splash_bm_bits` before `memcpy`.

The new `memset` avoids stale data when the splash image is smaller than the buffer, but `memcpy` still assumes `splash_size <= sizeof(badge_cfg.splash_bm_bits)`. If `ALIGN_1BYTE(splash.w) * splash.h` can exceed the destination size (e.g., due to config or asset changes), this will overflow. Please add a bound check (or clamp) on `splash_size` against `sizeof(badge_cfg.splash_bm_bits)` before calling `memcpy`.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}