OpenDMARC icon indicating copy to clipboard operation
OpenDMARC copied to clipboard

Format truncation into the header variable in opendmarc.c

Open bicknell opened this issue 4 years ago • 1 comments

On the develop branch, In opendmarc.c twice the "header" variable experiences format truncation:

Instance 1

opendmarc.c:3625:3: note: ‘snprintf’ output 32 or more bytes (assuming 2080) into a destination of size 1025
   snprintf(header, sizeof header,
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            "%s%s%s; dmarc=%s (p=%s dis=%s) header.from=%s",
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            authservid,
            ~~~~~~~~~~~
            conf->conf_authservidwithjobid ? "/" : "",
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            conf->conf_authservidwithjobid ? dfc->mctx_jobid : "",
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            aresult, apolicy, adisposition, dfc->mctx_fromdomain);
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Instance 2

opendmarc.c:3081:3: note: ‘snprintf’ output 31 or more bytes (assuming 2079) into a destination of size 1025
   snprintf(header, sizeof header,
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            "%s; dmarc=permerror header.from=%s",
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            authservid, dfc->mctx_fromdomain);

It appears to be used as a temporary variable for putting a new header in before adding it to the output. It is defined about line 2260:

        unsigned char header[MAXHEADER + 1];

The MAXHEADER length is defined in opendmarc.h:

#define MAXHEADER       1024

That define is used exactly one other spot, opendmarc-ar.c about line 714:

    u_char tmp[MAXHEADER + 2];

Note inconsistency with the +1 or +2.

Now, RFC2822 says:

   There are two limits that this standard places on the number of
   characters in a line. Each line of characters MUST be no more than
   998 characters, and SHOULD be no more than 78 characters, excluding
   the CRLF.

But also:

   Each header field is logically a single line of characters comprising
   the field name, the colon, and the field body.  For convenience
   however, and to deal with the 998/78 character limitations per line,
   the field body portion of a header field can be split into a multiple
   line representation; this is called "folding". 

Ergo, what I think needs to happen here is:

  1. The buffer needs to be made larger, probably on the order of 2048 characters.
  2. OpenDMARC needs to implement header folding if the output exceeds 998 characters.

bicknell avatar Mar 31 '21 12:03 bicknell

This is pretty minor, since all of the things that go into the header field have large-ish buffers but are almost always relatively tiny. Punting out at least to the next release.

mskucherawy avatar Apr 12 '21 06:04 mskucherawy