Potential Out-of-Bounds Read in v1ParseSrvTypeRqst Function

[hkbinbin]

In the function v1ParseSrvTypeRqst, there is a potential out-of-bounds read when processing the buffer.

code overview:

static int v1ParseSrvTypeRqst(const SLPBuffer buffer, int encoding,
      SLPSrvTypeRqst * srvtyperqst)
{
   int result;

   /* Enforce SLPv1 service type request size limits. */
   if (buffer->end - buffer->curpos < 6)
      return SLP_ERROR_PARSE_ERROR;

   /* Parse the <Previous Responders Addr Spec>, and convert to UTF-8. */
   srvtyperqst->prlistlen = GetUINT16(&buffer->curpos);
   srvtyperqst->prlist = GetStrPtr(&buffer->curpos, srvtyperqst->prlistlen);

   if (buffer->curpos > buffer->end) // ! if curpos == end
      return SLP_ERROR_PARSE_ERROR;

    ...

   /* Parse the <Naming Authority String>, and convert to UTF-8. */
   srvtyperqst->namingauthlen = GetUINT16(&buffer->curpos); // ! got nothing but curpos += 2
   if (!srvtyperqst->namingauthlen || srvtyperqst->namingauthlen == 0xffff)
        ...
   else
   {
      srvtyperqst->namingauth = GetStrPtr(&buffer->curpos,
            srvtyperqst->namingauthlen);
        ...
   }

   /* Parse the <Scope String>, and convert to UTF-8. */
   srvtyperqst->scopelistlen = GetUINT16(&buffer->curpos);
   if (srvtyperqst->scopelistlen)
   {
      srvtyperqst->scopelist = GetStrPtr(&buffer->curpos,
            srvtyperqst->scopelistlen);
        ...
   }
   else
   {
        ...
   }
   return 0;
}

After executing the following line:
srvtyperqst->prlist = GetStrPtr(&buffer->curpos, srvtyperqst->prlistlen);

There is a possibility that buffer->curpos has reached buffer->end, especially if prlistlen is crafted to consume all remaining data.

However, in the subsequent code:

srvtyperqst->namingauthlen = GetUINT16(&buffer->curpos);
...
srvtyperqst->scopelistlen = GetUINT16(&buffer->curpos);

Two consecutive GetUINT16 calls increase buffer->curpos by 4 bytes in total. If buffer->curpos was already at or beyond the buffer boundary, these reads will result in out-of-bounds access, potentially leading to undefined behavior or memory disclosure.

I also noticed that most functions performing memory operations on the buffer only validate bounds after the operation, typically using a check like:

if (buffer->curpos > buffer->end)

In at least one case, the code might reach a situation: buffer->curpos = buffer->end+1

   daadvert->spilist = GetStrPtr(&buffer->curpos, daadvert->spilistlen);
   if (buffer->curpos > buffer->end)
      return SLP_ERROR_PARSE_ERROR;

   /* Parse the authentication block list (if any). */
   daadvert->authcount = *buffer->curpos++;

This is mitigated by allocating the buffer with size + 1 bytes to prevent out-of-bounds access.

But in this specific function, there is no such check. It is a bug?