Reset to VARTYPE_NONE

[dsmith]

(Forgive the verbosity of this description. I copied it over from our internal repository as it was meant to educate those who were not very familiar with some of the workings of the underlying DA process)

During the deserialization of an object defined within a DA queue file, we will attempt to parse each property of an object in-order based on a ruleset. Each line within the object structure represents a property that is to be applied to the var struct. The type and values of the property are separated by :s. Here's an example of an object structure:

<Obj:1:msg:1:
+iProtocolVersion:2:1:0:
+iSeverity:2:1:5:
+iFacility:2:2:16:
+msgFlags:2:1:0:
+ttGenTime:2:10:1497509508:
+tRcvdAt:3:35:2:2017:6:14:23:51:48:435524:6:-:7:0:
+tTIMESTAMP:3:35:2:2017:6:14:23:51:48:435524:6:-:7:0:
+pszTAG:1:7:icinga::
+pszRawMsg:1:163:[1497449145] EXTERNAL COMMAND: PROCESS_SERVICE_CHECK_RESULT;z-packer-ami-591a937f-7c24-f95d-0ed8-a548dd26af72;Rsyslog Corrupt DA Queue;0;The DA is in working order:
+pszHOSTNAME:1:11:icinga-dev6:
+pszInputName:1:6:imfile:
+pszRcvFrom:1:0::
+pszRcvFromIP:1:0::
+localvars:1:18:{ "actionMod": 3 }:
+offMSG:2:1:0:
>End

A property of an object is represented by the set of characters that precede the first : and are after the +. +tTIMESTAMP:3:35:2:2017:6:14:23:51:48:435524:6:-:7:0: would equate to a tTIMESTAMP property who's type is 3 (VAR_SYSLOGTIMESTAMP). The characters after the next : represents the value for tTIMESTAMP, in this case 35:2:2017:6:14:23:51:48:435524:6:-:7.

In the case that a single line (property) does not confirm to the specific syntax, rsyslog will stop processing the object and attempt to cleanup anything it has already created. Of particular interest is the Destruct function of the var_t type.

/* destructor for the var object */
BEGINobjDestruct(var) /* be sure to specify the object type also in END and CODESTART macros! */
CODESTARTobjDestruct(var)
  if(pThis->pcsName != NULL)
    rsCStrDestruct(&pThis->pcsName);
  if(pThis->varType == VARTYPE_STR) {
    if(pThis->val.pStr != NULL)
      rsCStrDestruct(&pThis->val.pStr);
  }
ENDobjDestruct(var)

var_t is a structure that essentially represents a single property during serialization. A type enum is assigned with the appropriate value. If the type is VARTYPE_STR, the destruct function will do what is as expected which is to free up the memory allocated for the char * value that was assigned to it during deserialization. If the var_t struct is a number, for instance, there is nothing to do since we did not previously allocate any memory for the value.

Now, what happens when a property is deserialized that isn't correctly named? Let's take an object definition like the following:

<Obj:1:msg:1:
+iProtocolVersion:2:1:0:
+iSeverity:2:1:5:
+iFacility:2:2:16:
+msgFlags:2:1:0:
+ttGenTime:2:10:1497509508:
+tRcvdAt:3:35:2:2017:6:14:23:51:48:435524:6:-:7:0:
+tTIME<Obj:1:msg:1:
+iProtocolVersion:2:1:0:
+iSeverity:2:1:5:
+iFacility:2:2:16:
+msgFlags:2:1:0:
+ttGenTime:2:10:1497509508:
+tRcvdAt:3:35:2:2017:6:14:23:51:48:435524:6:-:7:0:
+tTIMESTAMP:3:35:2:2017:6:14:23:51:48:435524:6:-:7:0:
+pszTAG:1:7:icinga::
+pszRawMsg:1:163:[1497449145] EXTERNAL COMMAND: PROCESS_SERVICE_CHECK_RESULT;z-packer-ami-591a937f-7c24-f95d-0ed8-a548dd26af72;Rsyslog Corrupt DA Queue;0;The DA is in working order:
+pszHOSTNAME:1:11:icinga-dev6:
+pszInputName:1:6:imfile:
+pszRcvFrom:1:0::
+pszRcvFromIP:1:0::
+localvars:1:18:{ "actionMod": 3 }:
+offMSG:2:1:0:
>End

This is what an object looks like that would create a corrupt DA queue file. The line that will break the deserialization is +tTIME<Obj:1:msg:1:. This is because <Obj is the entry point for any new object.

When we attempt to deserialize the object, everything works out fine until we get to the property +tTIME<Obj:1:msg:1:. In the steps to deserialize the line, we will first grab the name (+tTIME<Obj) then grab the type, which is 1 or VARTYPE_STR and then the number of characters that represents the value of the string.

  /* get the property name first */
  CHKiRet(cstrConstruct(&pProp->pcsName));

  NEXTC;
  while(c != ':') {
    CHKiRet(cstrAppendChar(pProp->pcsName, c));
    NEXTC;
  }
  cstrFinalize(pProp->pcsName);
  step = 1;

  /* property type */
  CHKiRet(objDeserializeNumber(&i, pStrm));
  pProp->varType = i;
  step = 2;

  /* size (needed for strings) */
  CHKiRet(objDeserializeNumber(&iLen, pStrm));
  step = 3;

It is in step 3 of this use-case where the deserialization will fail. What is important to note here is that we've already assigned the var_t type variable pProp as a VARTYPE_STR. Since we failed to grab the size of the string from the value msg (makes sense!), we break out of our deserialization process and attempt to cleanup what we have already created.

Now, we're back to the Destruct function. Since the var_t struct is of type VARTYPE_STR will attempt to free up the memory allocated to the value of the var_t. But, in our deserialization process, we never got to that point. We now enter a place that is terrible for any C program which is attempting to free unallocated memory. At this point rsyslog dies without any indication as to what was going on.

My solution is to ensure that if we get through the deserialization process of a property without having completed extraction of the var_t value, we reset the type of the property to VARTYPE_NULL. That way when cleaning up after a deserialization failure, we do not attempt to free anything we do not own.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.