Fix code scan issues

[D-Triveni]

Fix the uncontrolled format string vlunerabilities and unsigned comparision fix.

Fixes SHEL-4009.

[Copilot]

Pull request overview

Addresses static code scan findings by hardening logging against format-string issues and fixing a signed/unsigned comparison in packet parsing (Fixes SHEL-4009).

Changes:

• Reworked indigo_logger() to use a fixed-size stack buffer and added snprintf() bounds checking.
• Updated TLV parsing loop condition in parse_packet() to avoid unsigned underflow / incorrect comparisons.

Reviewed changes

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

File	Description
utils.c	Changes logger formatting/buffer handling intended to mitigate format string issues.
indigo_packet.c	Fixes TLV parse loop condition to avoid signed/unsigned arithmetic pitfalls.

Comments suppressed due to low confidence (1)

utils.c:133

• Switching from a dynamically-sized buffer to MAX_FORMAT_LEN (256) means longer fmt strings will now drop the log entirely (ret >= sizeof(format) => return). If long format strings are expected (some call sites log large responses/buffers), consider either dynamically sizing like before or allowing truncation (and still printing the truncated message) to avoid losing diagnostics.

#define MAX_FORMAT_LEN 256

void indigo_logger(int level, const char *fmt, ...) {
    char format[MAX_FORMAT_LEN];
    char *log_type;
    int ret;
#ifdef _SYSLOG_
    int priority;
#endif
    va_list ap;

    switch (level) {
    case LOG_LEVEL_DEBUG_VERBOSE:
        log_type = "debugverbose";
        break;
    case LOG_LEVEL_DEBUG:
        log_type = "debug";
        break;
    case LOG_LEVEL_INFO:
        log_type = "info";
        break;
    case LOG_LEVEL_NOTICE:
        log_type = "notice";
        break;
    case LOG_LEVEL_WARNING:
        log_type = "warning";
        break;
    default:
        log_type = "info";
        break;
    }

    memset(format, 0, sizeof(format));
    ret = snprintf(format, sizeof(format), "controlappc.%8s  %s", log_type, fmt);
    if (ret < 0 || ret >= (int)sizeof(format)) {
        indigo_logger(LOG_LEVEL_ERROR, "snprintf failed or "
                                        "buffer overflow occurred at line %d "
                                        "- Return value: %d", __LINE__, ret);
        return;
    }

---

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

[Copilot]

The snprintf() failure path calls indigo_logger() again, which can recurse indefinitely (e.g., if the same formatting/size issue happens again) and potentially stack-overflow. Use a non-recursive fallback for internal logger errors (e.g., write to stderr/syslog with a constant format), or add a re-entrancy guard to prevent indigo_logger from calling itself.

[Copilot]

This still constructs a new format string that embeds fmt, then passes it to vprintf/vfprintf/vsyslog as the format (line 138/144/172). If fmt can contain % sequences, they will be interpreted, so the uncontrolled format string vulnerability is not actually eliminated. Safer pattern: render the user message with vsnprintf into a buffer, then print/log it using a constant format ("%s") alongside the prefix.

[Copilot]

req->tlv[req->tlv_num] is written without checking that req->tlv_num < TLV_NUM. A packet with many TLVs can drive tlv_num past the fixed array size (TLV_NUM=128) and cause out-of-bounds writes. Add a guard before allocating/storing the TLV and fail parsing (or stop) when the limit is reached.

Suggested change

	while ((size_t)parser < packet_len) {
	while ((size_t)parser < packet_len) {
	if (req->tlv_num >= TLV_NUM) {
	indigo_logger(LOG_LEVEL_ERROR, "%d: Exceeded maximum number of TLVs (%d)", __LINE__, TLV_NUM);
	return -1;
	}