Copy of Overlapping Memory in snprintf

[ArielSAdamsNASA]

Describe the bug
In the call to function snprintf, the object pointed to by argument local->volume_name may overlap with the object pointed to by argument local->system_mountpt.

local->system_mountpt is 64 bytes off of the address of OS_filesys_table[OS_ObjectIndexFromToken(token)].

local->volume_name is 32 bytes off of the address of OS_filesys_table[OS_ObjectIndexFromToken(token)]

Expected behavior
Resolve possible overlap.

Code snips

osal/src/os/posix/src/os-impl-filesys.c

Line 175 in 42af0f7

snprintf(local->system_mountpt, sizeof(local->system_mountpt), "%s/osal:%s", tmpdir, local->volume_name);

System observed on:
Coverity: https://scan.coverity.com/projects/arielsadamsnasa-cfs-jsf-rules?tab=overview

Reporter Info
Ariel Adams, ASRC Federal

[Jkillelea]

Hi all,

This bug just cropped up for me again, at commit 419c673.

The specific error is:

/home/[user]/projects/cFS/osal/src/os/posix/src/os-impl-filesys.c:173:9: error: ‘snprintf’ argument 5 overlaps destination object ‘OS_filesys_table’ [-Werror=restrict]
  173 |         snprintf(local->system_mountpt, sizeof(local->system_mountpt), "%s/osal:%s", tmpdir, local->volume_name);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[sunbrst]

Confirmed that I'm seeing this issue too (failing a build of cFS - https://github.com/nasa/cFS). Has anyone found a commit that works for this?

[jphickey]

I've looked at this before but it doesn't seem like a real issue. Yes - the volume_name and system_mountpt are members of the same struct, but they don't overlap. As long as volume_name is properly null-terminated within OS_FS_VOL_NAME_LEN bytes (32 by default, which it should be, I can't see any reason why it would not be) then this will not access overlapping memory.

I think the reason the compiler reports this warning is because if it was not null-terminated, it could theoretically write up to OS_MAX_LOCAL_PATH_LEN bytes into the destination, which is 128 by default. Because the pointers are only 32 bytes apart, this is why it gets flagged.

I am thinking of a possible workaround but in the meantime I'd suggest just adding -Wno-restrict to your compiler options.

[smparekh]

Thanks folks, I can confirm building of cFS on the latest commit.