[BUG] proto_smpp: off-by-one null byte write in copy_var_str

[jming912]

OpenSIPS version you are running

Latest master (commit a74fc46, 2026-03-19)

Describe the bug

The copy_var_str() function in modules/proto_smpp/utils.c has an off-by-one error that writes a null byte one byte past the destination buffer when the input string is exactly maxlen bytes long with no null terminator within that range.

// utils.c, line 37
int copy_var_str(char *to, char *from, int maxlen)
{
    int iret = 1;
    while (*from && maxlen--) {
        *to++ = *from++;
        iret++;
    }
    *to++ = '\0';   // writes null AFTER copying maxlen bytes → to[maxlen]
    return iret;
}

When the input contains maxlen or more non-null bytes:

1. The while loop copies exactly maxlen bytes into to[0..maxlen-1]
2. *to++ = '\0' writes a null byte at to[maxlen] — one byte past the end of the destination array

The most concerning call site is the last field of smpp_bind_receiver_t:

p += copy_var_str(body->address_range, p, MAX_ADDRESS_RANGE_LEN);

address_range is char[41], the last field of the struct. If the input has 41 non-null bytes, the null is written at address_range[41], past the struct boundary.

Note: issue #1867 reported a related but different symptom (field concatenation in the serialization path due to insufficient buffer sizes). This bug is in the deserialization/parsing path and was not addressed by that fix.

To Reproduce

// Compile: clang -fsanitize=address -g -o poc_obo poc_obo.c
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <stdint.h>

#define MAX_SYSTEM_ID_LEN 16
#define MAX_PASSWORD_LEN 9
#define MAX_SYSTEM_TYPE_LEN 13
#define MAX_ADDRESS_RANGE_LEN 41

typedef struct {
    char system_id[MAX_SYSTEM_ID_LEN];
    char password[MAX_PASSWORD_LEN];
    char system_type[MAX_SYSTEM_TYPE_LEN];
    uint8_t interface_version;
    uint8_t addr_ton;
    uint8_t addr_npi;
    char address_range[MAX_ADDRESS_RANGE_LEN];
} smpp_bind_receiver_t;

int copy_var_str(char *to, char *from, int maxlen) {
    int iret = 1;
    while (*from && maxlen--) {
        *to++ = *from++;
        iret++;
    }
    *to++ = '\0';
    return iret;
}

int main(void) {
    // Heap-allocate so ASAN detects the 1-byte write past the struct
    smpp_bind_receiver_t *body = malloc(sizeof(smpp_bind_receiver_t));
    memset(body, 'X', sizeof(smpp_bind_receiver_t));

    // Input: exactly MAX_ADDRESS_RANGE_LEN (41) non-null bytes
    char input[MAX_ADDRESS_RANGE_LEN];
    memset(input, 'A', MAX_ADDRESS_RANGE_LEN);

    // Writes null at body->address_range[41] — 1 byte past the struct
    copy_var_str(body->address_range, input, MAX_ADDRESS_RANGE_LEN);

    free(body);
    return 0;
}

Run:

$ clang -fsanitize=address -g -o poc_obo poc_obo.c && ./poc_obo
==ERROR: AddressSanitizer: heap-buffer-overflow on address ...
WRITE of size 1 at ... thread T0
    #0 in copy_var_str
    #1 in main
... is located 0 bytes after XX-byte region

Expected behavior

copy_var_str() should ensure the null terminator is written within the maxlen boundary, not after it. Either reserve 1 byte for the terminator or increase destination buffer sizes by 1.

Relevant System Logs

Without ASAN, a single null byte is silently written past the struct boundary, potentially corrupting heap metadata or adjacent allocations.

OS/environment information

• Operating System: Ubuntu 22.04
• OpenSIPS installation: git (master, commit a74fc46)

Additional context

• CWE: CWE-787 (Out-of-bounds Write), 1 null byte
• Attack vector: Network — send a crafted SMPP PDU with C-Octet-String fields exactly maxlen bytes long without a null terminator
• Authentication: Not required (bind PDU is the first message on a connection)
• Severity: Medium — writes 1 null byte past struct boundary on every copy_var_str call site where the input fills maxlen. Affects parse_bind_receiver_body(), parse_submit_or_deliver_body(), and all other callers.

Suggested fix:

int copy_var_str(char *to, char *from, int maxlen)
{
    int iret = 1;
    while (*from && maxlen > 1) {  // reserve 1 byte for null
        *to++ = *from++;
        iret++;
        maxlen--;
    }
    *to = '\0';
    if (*from) { while (*from) { from++; iret++; } iret++; }
    return iret;
}

[github-actions]

Any updates here? No progress has been made in the last 15 days, marking as stale. Will close this issue if no further updates are made in the next 30 days.