fix(network_provisioning): adds more checks for received messages

[Ashish285]

Checklist

• CI passing

Change description

This PR adds in more input validations and fixes some memory issues

1. Possible NULL dereference in malformed protobuf messages
2. Possible buffer overflow in Thread cmd_set_config_handler
3. Memory leak in Wifi scan result cleanup
4. Possible NULL dereference after malloc failure for network name

[Copilot]

Pull request overview

This PR hardens the network provisioning protobuf handlers by adding input validation for malformed/incorrectly-shaped messages and tightening up memory management during scan/config operations.

Changes:

• Added payload_case + pointer validation before dereferencing command payloads (WiFi/Thread scan + config set handlers).
• Prevented Thread dataset copy when the incoming dataset length exceeds the fixed buffer.
• Fixed a leak in WiFi scan result cleanup by freeing each allocated entry struct.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
network_provisioning/src/network_scan.c	Adds request payload validation; adds an OOM return in Thread scan result path; fixes WiFi scan result entry cleanup leak.
network_provisioning/src/network_config.c	Adds request payload validation for set-config commands; moves Thread dataset memcpy behind a length check to avoid overflow.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

network_provisioning/src/network_scan.c:259

• resp_payload->n_entries is set to the requested count before the loop, but the loop can break early on errors, leaving NULL (or partially initialized) entries while n_entries still indicates they are present. protobuf-c will attempt to pack all n_entries elements and can crash when it hits a NULL element or a bytes field with len>0 and data==NULL. Track the number of successfully built entries and set n_entries accordingly; on per-entry failure, free/NULL out the partially built entry and exclude it from n_entries.

        resp_payload->entries = results;
        resp_payload->n_entries = req->cmd_scan_wifi_result->count;

        /* If req->cmd_scan_wifi_result->count is 0, the below loop will
        * be skipped.
        */
        for (uint16_t i = 0; i < req->cmd_scan_wifi_result->count; i++) {
            if (!h->wifi_scan_result) {
                resp->status = STATUS__InternalError;
                break;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[AdityaHPatwardhan]

@Ashish285 minor comments, but otherwise MR LGTM.
Thanks for fixing and improving the error messages.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Ashish285]

I did some tests to check if the memory leak was indeed there. To create "memory leaks", I just set one buffer check after malloc to be always true, so it thinks that the malloc failed. These are the results

On current master:

1. First session: Heap before: 196448 after: 196400 delta: -48
2. Second: Heap before: 196328 after: 196284 delta: -44
3. Third: Heap before: 196232 after: 196172 delta: -60

Complete Logs

I (16177) app: Secured session established!
I (21417) proto_network_scan: Heap before: 197220 after: 196552 delta: -668
I (21587) proto_network_scan: Heap before: 196476 after: 196508 delta: 32
E (21787) proto_network_scan: Failed to allocate memory for scan result entry SSID
E (21787) proto_network_scan: Error executing command handler
E (21787) proto_network_scan: Command dispatcher error -1
E (21787) proto_network_scan: Unsupported response type in cleanup_handler
I (21797) proto_network_scan: Heap before: 196448 after: 196400 delta: -48
E (21807) protocomm: Request handler for prov-scan failed
E (21807) protocomm_nimble: Invalid content received, killing connection
W (25587) security2: Invalid state of session 0 (expected 2). Restarting session.
E (25587) security2: Closing old session with id 1
I (25587) security2: Using salt and verifier to generate public key...
I (26217) app: Secured session established!
I (31457) proto_network_scan: Heap before: 196344 after: 196392 delta: 48
I (31627) proto_network_scan: Heap before: 196364 after: 196396 delta: 32
E (31827) proto_network_scan: Failed to allocate memory for scan result entry SSID
E (31827) proto_network_scan: Error executing command handler
E (31827) proto_network_scan: Command dispatcher error -1
E (31837) proto_network_scan: Unsupported response type in cleanup_handler
I (31837) proto_network_scan: Heap before: 196328 after: 196284 delta: -44
E (31847) protocomm: Request handler for prov-scan failed
E (31847) protocomm_nimble: Invalid content received, killing connection
W (35087) security2: Invalid state of session 0 (expected 2). Restarting session.
E (35097) security2: Closing old session with id 1
I (35097) security2: Using salt and verifier to generate public key...
I (35727) app: Secured session established!
I (40967) proto_network_scan: Heap before: 196212 after: 196260 delta: 48
I (41137) proto_network_scan: Heap before: 196240 after: 196272 delta: 32
E (41337) proto_network_scan: Failed to allocate memory for scan result entry SSID
E (41337) proto_network_scan: Error executing command handler
E (41337) proto_network_scan: Command dispatcher error -1
E (41337) proto_network_scan: Unsupported response type in cleanup_handler
I (41347) proto_network_scan: Heap before: 196232 after: 196172 delta: -60
E (41357) protocomm: Request handler for prov-scan failed
E (41357) protocomm_nimble: Invalid content received, killing connection

With current branch

1. First session: Heap before: 196352 after: 196408 delta: 56
2. Second: Heap before: 196348 after: 196404 delta: 56
3. Third: Heap before: 196464 after: 196524 delta: 60

Complete Logs

I (16197) app: Secured session established!
I (21437) proto_network_scan: Heap before: 197220 after: 196456 delta: -764
I (21607) proto_network_scan: Heap before: 196380 after: 196412 delta: 32
E (21807) proto_network_scan: Failed to allocate memory for scan result entry SSID
E (21807) proto_network_scan: Error executing command handler
E (21807) proto_network_scan: Command dispatcher error -1
I (21807) proto_network_scan: Heap before: 196352 after: 196408 delta: 56
E (21817) protocomm: Request handler for prov-scan failed
E (21827) protocomm_nimble: Invalid content received, killing connection
W (28387) security2: Invalid state of session 0 (expected 2). Restarting session.
E (28387) security2: Closing old session with id 1
I (28387) security2: Using salt and verifier to generate public key...
I (29067) app: Secured session established!
I (34307) proto_network_scan: Heap before: 196352 after: 196384 delta: 32
I (34477) proto_network_scan: Heap before: 196348 after: 196392 delta: 44
E (34677) proto_network_scan: Failed to allocate memory for scan result entry SSID
E (34677) proto_network_scan: Error executing command handler
E (34677) proto_network_scan: Command dispatcher error -1
I (34677) proto_network_scan: Heap before: 196348 after: 196404 delta: 56
E (34687) protocomm: Request handler for prov-scan failed
E (34697) protocomm_nimble: Invalid content received, killing connection
W (45887) security2: Invalid state of session 0 (expected 2). Restarting session.
E (45887) security2: Closing old session with id 1
I (45887) security2: Using salt and verifier to generate public key...
I (46517) app: Secured session established!
I (51807) proto_network_scan: Heap before: 196360 after: 196508 delta: 148
I (51977) proto_network_scan: Heap before: 196464 after: 196508 delta: 44
E (52177) proto_network_scan: Failed to allocate memory for scan result entry SSID
E (52177) proto_network_scan: Error executing command handler
E (52177) proto_network_scan: Command dispatcher error -1
I (52177) proto_network_scan: Heap before: 196464 after: 196524 delta: 60
E (52187) protocomm: Request handler for prov-scan failed
E (52197) protocomm_nimble: Invalid content received, killing connection

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

network_provisioning/src/network_config.c:445

• In the Thread status cleanup path, ThreadAttachState.ext_pan_id.data is allocated in cmd_get_status_handler when attached, but it is never freed here. This will leak memory each time an attached Thread status response is built/cleaned up. Free thread_attached->ext_pan_id.data (when non-NULL) before freeing thread_attached itself.

        if (!resp->resp_get_thread_status) {
            break;
        }
#ifdef CONFIG_NETWORK_PROV_NETWORK_TYPE_THREAD
        switch (resp->resp_get_thread_status->thread_state) {
        case THREAD_NETWORK_STATE__Attaching:
            break;
        case THREAD_NETWORK_STATE__Attached:
            if (resp->resp_get_thread_status->thread_attached) {
                if (resp->resp_get_thread_status->thread_attached->name) {
                    free(resp->resp_get_thread_status->thread_attached->name);
                }
                free(resp->resp_get_thread_status->thread_attached);
            }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Harshal5]

LGTM!!

[mahavirj]

@wqx6 PTAL

[mahavirj]

@Ashish285 Please add change-log entry, bump component version

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.