Skip to content

Conversation

ventz
Copy link

@ventz ventz commented Aug 16, 2025

This PR provides a significant overhaul of the MeshCore LoRa repeater implementation,
focusing on security hardening, performance optimization, and improved reliability
across various embedded platforms.

Security Improvements

  • Implement login brute force protection with rate limiting (5 failed attempts in 60s)
  • Add anti-replay attack protection for path messages using hash-based history tracking
  • Validate path length inputs to prevent buffer overflow vulnerabilities
  • Reset failed login counters only after successful authentication
  • Add comprehensive bounds checking throughout all buffer operations
  • Warn about default password usage during setup
  • Implement proper input validation for all client data

Performance Optimizations

  • Reduce filesystem I/O overhead by maintaining persistent log file handles
  • Implement periodic file syncing (30s interval) rather than frequent close/reopen cycles
  • Optimize client table management to prioritize empty slots before LRU replacement
  • Add smart UI refresh logic that minimizes unnecessary display updates
  • Cache frequently used UI strings to reduce formatting overhead in display loop
  • Implement safe string handling with proper bounds checking throughout
  • Replace sprintf with snprintf for guaranteed buffer safety

Memory and Resource Management

  • Fix memory leak by replacing dynamic allocation (strdup) with stack-allocated buffers
  • Properly initialize all class members in constructors to prevent undefined behavior
  • Add comprehensive resource cleanup for open file handles
  • Implement proper struct packing for over-the-air data structures
  • Add explicit endianness handling for cross-platform compatibility
  • Add static_assert to verify critical struct sizes
  • Use FILE_O_APPEND flag to ensure proper log appending on all platforms

Code Robustness

  • Add millis() rollover-safe time comparisons
  • Implement button handling improvements with long-press detection
  • Refactor unsafe serial input handling with proper bounds checking
  • Add telemetry bounds checking to prevent potential buffer overflow
  • Improve error handling throughout the codebase
  • Implement smarter neighbor list formatting with proper bounds checking

@fdlamotte
Copy link
Collaborator

Lots of valuable work here, thanks

I would prefer having several PRs,

  • one for the UI (as its unrelated to the repeater code)
  • several ones for repeater
    • one for logging (and file management)
    • ...

on file management, I generally prefer closing files as soon as possible, we never know when resets will happen and how files will be treated (will the data be written on the flash or not ?). By closing files you explicitely tell to write them to the device

@ripplebiz
Copy link
Collaborator

Yes, I concur with fdlamotte. Better to submit smaller PR's that target something specific.
And, leaving file handles open indefinitely is, generally, not a good idea. For some Arduino environments, the number of concurrent, open file handles is as little as 2

@yg-ht
Copy link

yg-ht commented Aug 19, 2025

Sorry to be "that guy", but this has the feel of an LLM on it. I am more than OK with the use of LLMs, however, for me if it feels like it is an LLM, it means there hasn't been enough human-in-the-loop.

I suspect if you split these into individual PRs and include details about testing, implications for the protocol spec, a discussion about any future problems and so on, it would mean that each of these significant changes can be considered as a distinct component.

At the moment, I worry that accepting this PR would be far too sweeping and may introduce issues in unexpected places and that this would then become difficult to unpick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants