libselinux: off-by-one in setcon() family

[enh-google]

setprocattrcon_raw() writes the C terminating '\0' (https://github.com/SELinuxProject/selinux/blob/main/libselinux/src/procattr.c#L217):

   ret = write(fd, context2, strlen(context2) + 1);

which i assume is an accident?

for C programs using libselinux, the strdup() in getprocattrcon_raw() at https://github.com/SELinuxProject/selinux/blob/main/libselinux/src/procattr.c#L160 hides this by truncating at the '\0', so callers appear to read back what they wrote (even if the kernel saw a slightly different byte sequence).

anything reading /proc directly in a language that can represent a '\0' in a string will see this extra '\0', despite it not intentionally being part of the context.

there's some weird-looking code in the kernel too:

        /* Obtain a SID for the context, if one was specified. */
        if (size && str[0] && str[0] != '\n') {
                if (str[size-1] == '\n') {
                        str[size-1] = 0;
                        size--;
                }
                error = security_context_to_sid(value, size,
                                                &sid, GFP_KERNEL);
                if (error == -EINVAL && !strcmp(name, "fscreate")) {
                        if (!has_cap_mac_admin(true)) {
                                struct audit_buffer *ab;
                                size_t audit_size;

                                /* We strip a nul only if it is at the end, otherwise the
                                 * context contains a nul and we should audit that */
                                if (str[size - 1] == '\0')
                                        audit_size = size - 1;
                                else
                                        audit_size = size;

if contexts are allowed to contain '\0's, isn't the libselinux getter API inherently broken? it'll truncate the context at the first '\0', which seems suboptimal. but it seems to me that contexts are compared in the kernel using strcmp() rather than memcmp(), so allowing '\0' in a context seems like a bug?

(i'm looking at this because i have code that just records the selinux context of a crashing process in a crash dump, and the '\0's that i would argue are inserted by libselinux are showing up there. given that it's not clear to me that the libselinux code is correct or intended, i didn't want to just duplicate the "truncate at the first '\0'" logic without at least filing a bug...)