Re: Suspected off-by-one in context_struct_to_string()

[Willy Tarreau <w@1wt.eu>]

Hi Stephen,

On Fri, Jan 16, 2026 at 10:12:15AM -0500, Stephen Smalley wrote:
> On Fri, Jan 16, 2026 at 3:16 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > On Thu, 15 Jan 2026 at 21:20, Willy Tarreau <w@1wt.eu> wrote:
> > >
> > > Hello,
> > >
> > > we've received a suspected vulnerability report on the kernel security
> > > list, that was clearly generated by AI and really not clear at all on
> > > the root causes nor impacts. We first dismissed it and it kept coming
> > > back a few times. I'm not pasting it because it's more confusing than
> > > interesting, though I can pass it to the maintainers if desired. I'm
> > > also purposely *not* CCing the reporter, as the address changed a few
> > > times, and once you respond you receive a new copy of the same report.
> > > Clearly this bot deserves a bit more tuning.
> > >
> > > The report claimed that the call to mls_compute_context_len() didn't
> > > properly reflect the size needed by mls_sid_to_context() due to an
> > > off-by-one that would result in the trailing zero being written too far.
> > > Initially we thought that was wrong since there are +1 everywhere in
> > > all lengths calculation in the function. But revisiting it today made
> > > us realize that this indeed seems to be true: the +1 that are everywhere
> > > are in fact due to the surrounding delimiters, and the first one that
> > > appeared to be the one accounting for the trailing zero was in fact
> > > for the starting colon.
> > >
> > > In context_struct_to_string(), we have this:
> > >
> > >         *scontext_len += strlen(sym_name(p, SYM_USERS, context->user - 1)) + 1;
> > >         *scontext_len += strlen(sym_name(p, SYM_ROLES, context->role - 1)) + 1;
> > >         *scontext_len += strlen(sym_name(p, SYM_TYPES, context->type - 1)) + 1;
> >
> > I think this +1 from the type name length covers the trailing NUL
> > byte, since mls_compute_context_len() and mls_sid_to_context() cover
> > the one byte space for the separating colon between type and optional
> > MLS component.
> 
> That is correct. The MLS portion is conditional on whether MLS is
> enabled; hence, the +1 for the type length computation includes the
> terminating NUL,
> and then the mls_compute_context_len() starts at 1 for the leading ":"
> if MLS is enabled. The code could admittedly be clearer but I don't
> believe this is a bug.

OK and I, too, think that mls_compute_context_len() stands by what
its name implies. But then *who* is responsible in all this chain
for allocating the room for the trailing zero that is being appended
at the end ?

Or is this the last +1 of this block maybe ?

     *scontext_len += strlen(sym_name(p, SYM_USERS, context->user - 1)) + 1; // ':'
     *scontext_len += strlen(sym_name(p, SYM_ROLES, context->role - 1)) + 1; // ':'
     *scontext_len += strlen(sym_name(p, SYM_TYPES, context->type - 1)) + 1; // \0 ?

I'm asking because nothing is really clear, and if it happens to work as
intended, it's not super clear why.

thanks,
Willy