Compare commits

...

9 Commits

Author SHA1 Message Date
Ben Meadors
c851861a36 Merge branch 'master' into nodenum-consistency 2025-08-11 21:22:47 -05:00
renovate[bot]
a2df80e833 chore(deps): update actions/checkout action to v5 (#7605)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
2025-08-11 20:58:54 -05:00
Ben Meadors
db238ef524 Log when this happened 2025-08-11 19:49:35 -05:00
Jonathan Bennett
f2b935f48f Stop the bleeding with malicious NodeDB overwrites (#7596) 2025-08-11 15:52:28 -05:00
Thomas Göttgens
e69da71d4e reorder for correct recognition (#7604) 2025-08-11 11:53:01 +02:00
Ben Meadors
ed4a30e526 Add transport 2025-08-10 08:38:44 -05:00
Ben Meadors
b1c5f871b6 Failing test should pass now with Jonathan's fix 2025-08-10 08:22:37 -05:00
Jonathan Bennett
683fb206a6 Merge branch 'master' into nodenum-consistency 2025-08-09 13:59:25 -05:00
Jonathan Bennett
573fb47b45 Only ever reset NodeNum back to the number derived from the MAC Address 2025-08-09 13:43:24 -05:00
24 changed files with 208 additions and 45 deletions

View File

@@ -5,7 +5,7 @@ runs:
using: composite
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
submodules: recursive
ref: ${{github.event.pull_request.head.ref}}

View File

@@ -24,7 +24,7 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
submodules: recursive
path: meshtasticd

View File

@@ -20,7 +20,7 @@ jobs:
name: build-${{ inputs.platform }}
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4
- uses: actions/checkout@v5
with:
submodules: recursive
ref: ${{github.event.pull_request.head.ref}}

View File

@@ -47,7 +47,7 @@ jobs:
runs-on: ${{ inputs.runs-on }}
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
submodules: recursive
ref: ${{github.event.pull_request.head.ref}}

View File

@@ -83,7 +83,7 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
submodules: recursive
ref: ${{github.event.pull_request.head.ref}}

View File

@@ -19,7 +19,7 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
submodules: recursive
ref: ${{ github.ref }}

View File

@@ -42,7 +42,7 @@ jobs:
- check
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4
- uses: actions/checkout@v5
- uses: actions/setup-python@v5
with:
python-version: 3.x
@@ -72,7 +72,7 @@ jobs:
version:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/checkout@v5
- name: Get release version string
run: |
echo "long=$(./bin/buildinfo.py long)" >> $GITHUB_OUTPUT
@@ -93,7 +93,7 @@ jobs:
runs-on: ubuntu-latest
if: ${{ github.event_name != 'workflow_dispatch' }}
steps:
- uses: actions/checkout@v4
- uses: actions/checkout@v5
- name: Build base
id: base
uses: ./.github/actions/setup-base
@@ -288,7 +288,7 @@ jobs:
]
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
ref: ${{github.event.pull_request.head.ref}}
repository: ${{github.event.pull_request.head.repo.full_name}}
@@ -367,7 +367,7 @@ jobs:
- package-pio-deps-native-tft
steps:
- name: Checkout
uses: actions/checkout@v4
uses: actions/checkout@v5
- name: Setup Python
uses: actions/setup-python@v5
@@ -436,7 +436,7 @@ jobs:
needs: [release-artifacts, version]
steps:
- name: Checkout
uses: actions/checkout@v4
uses: actions/checkout@v5
- name: Setup Python
uses: actions/setup-python@v5
@@ -491,7 +491,7 @@ jobs:
esp32,esp32s3,esp32c3,esp32c6,nrf52840,rp2040,rp2350,stm32
steps:
- name: Checkout
uses: actions/checkout@v4
uses: actions/checkout@v5
- name: Setup Python
uses: actions/setup-python@v5

View File

@@ -14,7 +14,7 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4
uses: actions/checkout@v5
- name: Trunk Check
uses: trunk-io/trunk-action@v1
@@ -31,7 +31,7 @@ jobs:
pull-requests: write # For trunk to create PRs
steps:
- name: Checkout
uses: actions/checkout@v4
uses: actions/checkout@v5
- name: Trunk Upgrade
uses: trunk-io/trunk-action/upgrade@v1

View File

@@ -34,7 +34,7 @@ jobs:
needs: build-debian-src
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
submodules: recursive
path: meshtasticd

View File

@@ -24,7 +24,7 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
submodules: recursive
ref: ${{github.event.pull_request.head.ref}}

View File

@@ -32,7 +32,7 @@ jobs:
needs: build-debian-src
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
submodules: recursive
path: meshtasticd

View File

@@ -60,7 +60,7 @@ jobs:
shell: bash
steps:
- name: Checkout
uses: actions/checkout@v4
uses: actions/checkout@v5
- name: Setup Python
uses: actions/setup-python@v5

View File

@@ -21,7 +21,7 @@ jobs:
steps:
# step 1
- name: clone application source code
uses: actions/checkout@v4
uses: actions/checkout@v5
# step 2
- name: full scan

View File

@@ -13,7 +13,7 @@ jobs:
steps:
# step 1
- name: clone application source code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
fetch-depth: 0

View File

@@ -14,7 +14,7 @@ jobs:
name: Native Simulator Tests
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/checkout@v5
with:
ref: ${{github.event.pull_request.head.ref}}
repository: ${{github.event.pull_request.head.repo.full_name}}
@@ -70,7 +70,7 @@ jobs:
name: Native PlatformIO Tests
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/checkout@v5
with:
ref: ${{github.event.pull_request.head.ref}}
repository: ${{github.event.pull_request.head.repo.full_name}}
@@ -127,7 +127,7 @@ jobs:
- platformio-tests
if: always()
steps:
- uses: actions/checkout@v4
- uses: actions/checkout@v5
with:
ref: ${{github.event.pull_request.head.ref}}
repository: ${{github.event.pull_request.head.repo.full_name}}

View File

@@ -20,7 +20,7 @@ jobs:
runs-on: test-runner
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
# - uses: actions/setup-python@v5
# with:

View File

@@ -18,7 +18,7 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4
uses: actions/checkout@v5
- name: Trunk Check
uses: trunk-io/trunk-action@v1

View File

@@ -16,7 +16,7 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4
uses: actions/checkout@v5
- name: Trunk Check
uses: trunk-io/trunk-action@v1

View File

@@ -15,7 +15,7 @@ jobs:
pull-requests: write
steps:
- name: Checkout repository
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
ref: ${{github.event.pull_request.head.ref}}
repository: ${{github.event.pull_request.head.repo.full_name}}

View File

@@ -11,7 +11,7 @@ jobs:
pull-requests: write
steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@v5
with:
submodules: true

View File

@@ -985,8 +985,9 @@ void NodeDB::resetNodes()
void NodeDB::removeNodeByNum(NodeNum nodeNum)
{
int newPos = 0, removed = 0;
for (int i = 0; i < numMeshNodes; i++) {
// Don't remove the own node at position 0
int newPos = 1, removed = 0;
for (int i = 1; i < numMeshNodes; i++) {
if (meshNodes->at(i).num != nodeNum)
meshNodes->at(newPos++) = meshNodes->at(i);
else
@@ -1082,18 +1083,16 @@ void NodeDB::pickNewNodeNum()
}
meshtastic_NodeInfoLite *found;
while (((found = getMeshNode(nodeNum)) && memcmp(found->user.macaddr, ourMacAddr, sizeof(ourMacAddr)) != 0) ||
(nodeNum == NODENUM_BROADCAST || nodeNum < NUM_RESERVED)) {
NodeNum candidate = random(NUM_RESERVED, LONG_MAX); // try a new random choice
if (found)
LOG_WARN("NOTE! Our desired nodenum 0x%x is invalid or in use, by MAC ending in 0x%02x%02x vs our 0x%02x%02x, so "
"trying for 0x%x",
nodeNum, found->user.macaddr[4], found->user.macaddr[5], ourMacAddr[4], ourMacAddr[5], candidate);
nodeNum = candidate;
if (((found = getMeshNode(nodeNum)) && memcmp(found->user.macaddr, ourMacAddr, sizeof(ourMacAddr)) != 0) ||
(nodeNum == NODENUM_BROADCAST || nodeNum < NUM_RESERVED)) {
NodeNum newNodeNum = (ourMacAddr[2] << 24) | (ourMacAddr[3] << 16) | (ourMacAddr[4] << 8) | ourMacAddr[5];
LOG_WARN("NOTE! Our saved nodenum 0x%x is invalid or in use. Using 0x%x", nodeNum, newNodeNum);
nodeNum = newNodeNum;
}
LOG_DEBUG("Use nodenum 0x%x ", nodeNum);
myNodeInfo.my_node_num = nodeNum;
removeNodeByNum(nodeNum); // Since we skip 0, this should only ever remove outside matches.
}
/** Load a protobuf from a file, return LoadFileResult */
@@ -1689,10 +1688,10 @@ bool NodeDB::updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelInde
/// we updateGUI and updateGUIforNode if we think our this change is big enough for a redraw
void NodeDB::updateFrom(const meshtastic_MeshPacket &mp)
{
// if (mp.from == getNodeNum()) {
// LOG_DEBUG("Ignore update from self");
// return;
// }
if (mp.transport_mechanism != meshtastic_MeshPacket_TransportMechanism_TRANSPORT_API && mp.from == getNodeNum()) {
LOG_DEBUG("Ignore update from self");
return;
}
if (mp.which_payload_variant == meshtastic_MeshPacket_decoded_tag && mp.from) {
LOG_DEBUG("Update DB node 0x%x, rx_time=%u", mp.from, mp.rx_time);

View File

@@ -14,6 +14,10 @@ bool NodeInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mes
{
auto p = *pptr;
if (mp.from == nodeDB->getNodeNum()) {
LOG_WARN("Ignoring packet supposed to be from our own node: %08x", mp.from);
return false;
}
if (p.is_licensed != owner.is_licensed) {
LOG_WARN("Invalid nodeInfo detected, is_licensed mismatch!");
return true;

View File

@@ -53,6 +53,9 @@
#define HW_VENDOR meshtastic_HardwareModel_WISMESH_TAG
#elif defined(GAT562_MESH_TRIAL_TRACKER)
#define HW_VENDOR meshtastic_HardwareModel_GAT562_MESH_TRIAL_TRACKER
#elif defined(NOMADSTAR_METEOR_PRO)
#define HW_VENDOR meshtastic_HardwareModel_NOMADSTAR_METEOR_PRO
// MAke sure all custom RAK4630 boards are defined before the generic RAK4630
#elif defined(RAK4630)
#define HW_VENDOR meshtastic_HardwareModel_RAK4631
#elif defined(TTGO_T_ECHO)
@@ -89,8 +92,6 @@
#define HW_VENDOR meshtastic_HardwareModel_SEEED_SOLAR_NODE
#elif defined(HELTEC_MESH_POCKET)
#define HW_VENDOR meshtastic_HardwareModel_HELTEC_MESH_POCKET
#elif defined(NOMADSTAR_METEOR_PRO)
#define HW_VENDOR meshtastic_HardwareModel_NOMADSTAR_METEOR_PRO
#elif defined(SEEED_WIO_TRACKER_L1_EINK)
#define HW_VENDOR meshtastic_HardwareModel_SEEED_WIO_TRACKER_L1_EINK
#elif defined(SEEED_WIO_TRACKER_L1)

View File

@@ -0,0 +1,159 @@
#include "DebugConfiguration.h"
#include "TestUtil.h"
#include <unity.h>
#ifdef ARCH_PORTDUINO
#include "mesh/NodeDB.h"
#include "mesh/generated/meshtastic/mesh.pb.h"
#include "modules/NodeInfoModule.h"
// Mock NodeDB that tracks when updateUser would be called - follows MQTT test pattern
class MockNodeDB : public NodeDB
{
public:
MockNodeDB()
{
updateUserCallCount = 0;
lastUpdatedNodeNum = 0;
}
// Override virtual getMeshNode method (same as MQTT test pattern)
meshtastic_NodeInfoLite *getMeshNode(NodeNum n) override { return &emptyNode; }
// Track calls that would go to updateUser (we'll check this in the test)
// Since updateUser is not virtual, we override a method that's called during the process
meshtastic_NodeInfoLite *getMeshNodeForUpdate(NodeNum n)
{
updateUserCallCount++;
lastUpdatedNodeNum = n;
return &emptyNode;
}
int updateUserCallCount;
NodeNum lastUpdatedNodeNum;
meshtastic_NodeInfoLite emptyNode = {};
};
// Testable version of NodeInfoModule that exposes protected methods
class TestableNodeInfoModule : public NodeInfoModule
{
public:
bool testHandleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshtastic_User *user)
{
return handleReceivedProtobuf(mp, user);
}
};
void test_nodeinfo_spoofing_vulnerability()
{
// Create mock NodeDB and assign to global pointer like MQTT test
const std::unique_ptr<MockNodeDB> mockNodeDB(new MockNodeDB());
nodeDB = mockNodeDB.get();
// Set our node number (simulating what happens in real startup)
myNodeInfo.my_node_num = 0x12345678;
// Create a test NodeInfoModule
TestableNodeInfoModule testModule;
// Create a spoofed packet claiming to be from our own node
meshtastic_MeshPacket spoofedPacket = meshtastic_MeshPacket_init_default;
spoofedPacket.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA;
spoofedPacket.from = 0x12345678; // VULNERABILITY: Same as our node number
spoofedPacket.to = NODENUM_BROADCAST;
spoofedPacket.channel = 0;
spoofedPacket.which_payload_variant = meshtastic_MeshPacket_decoded_tag;
spoofedPacket.decoded.portnum = meshtastic_PortNum_NODEINFO_APP;
// Create malicious User data that an attacker wants to inject
meshtastic_User maliciousUser = meshtastic_User_init_default;
strcpy(maliciousUser.long_name, "HACKED_NODE");
strcpy(maliciousUser.short_name, "HAK");
strcpy(maliciousUser.id, "!87654321"); // Attacker's fake ID
maliciousUser.is_licensed = true; // Try to make us appear licensed when we're not
// Test the vulnerability: handleReceivedProtobuf should reject spoofed packets claiming to be from our own node
// but currently it processes them, calling updateUser with our own node number
bool result = testModule.testHandleReceivedProtobuf(spoofedPacket, &maliciousUser);
// The vulnerability is demonstrated by the function NOT rejecting the spoofed packet
// In a secure implementation, packets claiming to be from our own node should be rejected
// and the function should return true (meaning "I handled this, don't process further")
// Currently this will FAIL because the vulnerability exists:
// - The function returns false (allowing further processing)
// - It calls updateUser with the spoofed node number (our own number)
// - This allows an attacker to modify our node information
TEST_ASSERT_FALSE_MESSAGE(result,
"VULNERABILITY CONFIRMED: handleReceivedProtobuf processes spoofed packets from our own node.\n"
"Expected: Function should return true (reject spoofed packet)\n"
"Actual: Function returned false (processed spoofed packet)\n"
"This allows attackers to spoof our node number and modify our NodeInfo.");
printf("\n=== SECURITY TEST RESULTS ===\n");
printf("✗ Vulnerability exists: NodeInfoModule processes spoofed packets from our own node\n");
printf("✗ Attack vector: Attacker can spoof packets with from=our_node_number\n");
printf("==============================\n\n");
}
void test_legitimate_packet_processing()
{
// Test that legitimate packets from OTHER nodes are processed correctly
const std::unique_ptr<MockNodeDB> mockNodeDB(new MockNodeDB());
nodeDB = mockNodeDB.get();
myNodeInfo.my_node_num = 0x12345678;
TestableNodeInfoModule testModule;
// Create a legitimate packet from a DIFFERENT node
meshtastic_MeshPacket legitimatePacket = meshtastic_MeshPacket_init_default;
legitimatePacket.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA;
legitimatePacket.from = 0x87654321; // Different node number - this is legitimate
legitimatePacket.to = NODENUM_BROADCAST;
legitimatePacket.channel = 0;
legitimatePacket.which_payload_variant = meshtastic_MeshPacket_decoded_tag;
legitimatePacket.decoded.portnum = meshtastic_PortNum_NODEINFO_APP;
meshtastic_User legitimateUser = meshtastic_User_init_default;
strcpy(legitimateUser.long_name, "Legitimate User");
strcpy(legitimateUser.short_name, "LEG");
bool result = testModule.testHandleReceivedProtobuf(legitimatePacket, &legitimateUser);
// Legitimate packets should be processed normally (return false for further processing)
TEST_ASSERT_FALSE_MESSAGE(result, "Legitimate packets from other nodes should be processed normally");
printf("✓ Legitimate packet processing works correctly\n");
}
void setUp()
{
// Required by Unity
}
void tearDown()
{
// Required by Unity
}
void setup()
{
// Initialize test environment like MQTT test
initializeTestEnvironment();
UNITY_BEGIN();
printf("\n=== NodeInfo Spoofing Security Test ===\n");
printf("Testing vulnerability in NodeInfoModule::handleReceivedProtobuf()\n");
printf("Issue: Function doesn't check if packet claims to be from our own node\n\n");
RUN_TEST(test_nodeinfo_spoofing_vulnerability);
RUN_TEST(test_legitimate_packet_processing);
UNITY_END();
}
void loop() {}
#endif