hello tech@, this was reported some time ago on the OpenSMTPD-portable repository[0]
[0]: https://github.com/OpenSMTPD/OpenSMTPD/pull/1192 Filters can register to the data-line event to alter the mail content. smtpd, when parsing the filter' output it first copies the received line in a temporary buffer. Here truncation may happen if a line of the mail is longer than LINE_MAX (minus 50 to be exact). The proposed diff on github bumps the local buffer to SMTP_LINE_MAX + 256, but I think it's quite wasteful to have a buffer that long on the stack (SMTP_LINE_MAX is 65535 bytes). A very simple fix could be: : if (strcmp(kind, "filter-dataline") == 0) { : if (fs->phase != FILTER_DATA_LINE) : fatalx("filter-dataline out of dataline phase"); : - filter_data_next(token, reqid, response); : + filter_data_next(token, reqid, line + (response - buffer)); : return; : } : if (fs->phase == FILTER_DATA_LINE) since `line' contains the original line and it's guaranteed that the fields of the reply excluding the last (variable width) fits the buffer, it works. maybe it's too clever. Then I realized that we don't need to copy the line at all. We're already using strtoull to parse the number and the payload is the last field of the received line, so we can just advance the pointer. The drawback is that we now need to use a few strncmp, but I think it's worth it. disclaimer: i've run this only on localhost so far. To reproduce, you can use an awk script like the following #!/usr/bin/awk -f # filter-dummy: copies the data lines as-is. BEGIN { FS = "|" } /^config\|ready$/ { print "register|filter|smtp-in|data-line" print "register|ready" fflush() } "filter|smtp-in|data-line" == $1"|"$4"|"$5 { line = substr($0, length($1$2$3$4$5$6$7) + 8) printf "filter-dataline|%s|%s|%s\n", $6, $7, line fflush() } with a configuration like: table aliases file:/etc/mail/aliases filter "dummy" proc-exec "/path/to/filter-dummy" listen on lo0 filter "dummy" action "local" mbox alias <aliases> match from local for local action "local" and then sending a mail with a very long line: % tr -cd '[:print:]' </dev/random | dd bs=2048 count=16 | smtp $USER thoughts? diff /usr/src commit - 5c586f5f5360442b12bbc4ea18ce006ea0c3d126 path + /usr/src blob - a714446c26fee299f4450ff1ad40289b5b327824 file + usr.sbin/smtpd/lka_filter.c --- usr.sbin/smtpd/lka_filter.c +++ usr.sbin/smtpd/lka_filter.c @@ -593,40 +593,27 @@ lka_filter_process_response(const char *name, const ch { uint64_t reqid; uint64_t token; - char buffer[LINE_MAX]; char *ep = NULL; - char *kind = NULL; - char *qid = NULL; - /*char *phase = NULL;*/ - char *response = NULL; - char *parameter = NULL; + const char *kind = NULL; + const char *qid = NULL; + const char *response = NULL; + const char *parameter = NULL; struct filter_session *fs; - (void)strlcpy(buffer, line, sizeof buffer); - if ((ep = strchr(buffer, '|')) == NULL) + kind = line; + + if ((ep = strchr(kind, '|')) == NULL) fatalx("Missing token: %s", line); - ep[0] = '\0'; - - kind = buffer; - qid = ep+1; - if ((ep = strchr(qid, '|')) == NULL) - fatalx("Missing reqid: %s", line); - ep[0] = '\0'; - reqid = strtoull(qid, &ep, 16); - if (qid[0] == '\0' || *ep != '\0') + if (qid[0] == '\0' || *ep != '|') fatalx("Invalid reqid: %s", line); if (errno == ERANGE && reqid == ULLONG_MAX) fatal("Invalid reqid: %s", line); - qid = ep+1; - if ((ep = strchr(qid, '|')) == NULL) - fatal("Missing directive: %s", line); - ep[0] = '\0'; - + qid = ep + 1; token = strtoull(qid, &ep, 16); - if (qid[0] == '\0' || *ep != '\0') + if (qid[0] == '\0' || *ep != '|') fatalx("Invalid token: %s", line); if (errno == ERANGE && token == ULLONG_MAX) fatal("Invalid token: %s", line); @@ -637,7 +624,7 @@ lka_filter_process_response(const char *name, const ch if ((fs = tree_get(&sessions, reqid)) == NULL) return; - if (strcmp(kind, "filter-dataline") == 0) { + if (strncmp(kind, "filter-dataline|", 16) == 0) { if (fs->phase != FILTER_DATA_LINE) fatalx("filter-dataline out of dataline phase"); filter_data_next(token, reqid, response); @@ -646,19 +633,13 @@ lka_filter_process_response(const char *name, const ch if (fs->phase == FILTER_DATA_LINE) fatalx("filter-result in dataline phase"); - if ((ep = strchr(response, '|'))) { + if ((ep = strchr(response, '|')) != NULL) parameter = ep + 1; - ep[0] = '\0'; - } if (strcmp(response, "proceed") == 0) { - if (parameter != NULL) - fatalx("Unexpected parameter after proceed: %s", line); filter_protocol_next(token, reqid, 0); return; } else if (strcmp(response, "junk") == 0) { - if (parameter != NULL) - fatalx("Unexpected parameter after junk: %s", line); if (fs->phase == FILTER_COMMIT) fatalx("filter-reponse junk after DATA"); filter_result_junk(reqid); @@ -667,11 +648,11 @@ lka_filter_process_response(const char *name, const ch if (parameter == NULL) fatalx("Missing parameter: %s", line); - if (strcmp(response, "rewrite") == 0) + if (strncmp(response, "rewrite|", 8) == 0) filter_result_rewrite(reqid, parameter); - else if (strcmp(response, "reject") == 0) + else if (strncmp(response, "reject|", 7) == 0) filter_result_reject(reqid, parameter); - else if (strcmp(response, "disconnect") == 0) + else if (strncmp(response, "disconnect|", 11) == 0) filter_result_disconnect(reqid, parameter); else fatalx("Invalid directive: %s", line);
