From 17589132c2e0efcfff1ffd23ff28ea2f73220091 Mon Sep 17 00:00:00 2001 From: Pongsathon Sirithanyakul Date: Wed, 23 Sep 2026 00:07:45 +0700 Subject: [PATCH] ppd-generator: escape control chars in IPP string attributes emitted into PPD Route printer-make-and-model through a new ppd_escape_string() helper before embedding into *Manufacturer, *ModelName, *Product, *NickName, and *ShortNickName. This prevents an attacker-influenceable IPP attribute (from an LAN mDNS-discovered printer picked up by cups-browsed) that contains a raw newline, double-quote, or backslash from breaking out of its quoted PPD string context and appearing as additional PPD directives in the generated file. This is a defense-in-depth fix. No downstream execution of injected directives is demonstrated on the current cups-filters 1.28.17 (Debian bookworm) foomatic-rip parser, because its first-declaration-wins semantics keep the injected directive dormant behind the empty baseline template entry. The escape discipline belongs in the emitter regardless: parser semantics are not a contract the PPD generator can rely on across distros or future refactors. Add regression tests T08b-T08d in ppd/test_ppd_generator.c that assert: T08b \n in printer-make-and-model is squashed to a space; no second '*directive' line appears in the output. T08c " in printer-make-and-model is backslash-escaped. T08d \ in printer-make-and-model is doubled. The tests deliberately do NOT assert any specific downstream parser behaviour; they lock in the emitter's escape discipline so future refactors do not silently reopen the gap. Context: reported as GHSA-5wv7-gqmh-2pf2 on the cups-filters repository (routed here by upstream per zdohnal's feedback). No CVE is requested; the disclosure is a defense-in-depth hardening PR against the class of missing-blacklist-char bugs previously fixed piecewise in filter/foomatic-rip/util.c (CVE-2015-8327 backtick, CVE-2015-8560 semicolon). Reporter: Pongsathon Sirithanyakul Co-credit: Warunyou Sunpachit, Khamolwan Hnunainam (IT SELECT LAB Co., Ltd., Thailand) --- ppd/ppd-generator.c | 94 +++++++++++++++++++++++++++++++++++--- ppd/test_ppd_generator.c | 98 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 185 insertions(+), 7 deletions(-) diff --git a/ppd/ppd-generator.c b/ppd/ppd-generator.c index 3ba8c7ee..3edcc646 100644 --- a/ppd/ppd-generator.c +++ b/ppd/ppd-generator.c @@ -179,8 +179,74 @@ ppdCreatePPDFromIPP(char *buffer, // I - Filename buffer // IPP record for printer clusters // +// +// 'ppd_escape_string()' - Escape a value for safe embedding inside a +// PPD double-quoted string. +// +// PPD directives use the syntax `*Keyword: "value"`. A raw newline, +// unescaped double-quote, or backslash inside `value` allows the value +// to break out of its containing string and be re-interpreted as new +// PPD directives by a downstream PPD parser. Every callsite that +// embeds an attacker-influenceable IPP string attribute +// (printer-make-and-model, printer-info, printer-location, printer-name) +// into a quoted PPD directive must run through this function first. +// +// Transformations: +// backslash -> backslash backslash +// double-quote -> backslash double-quote +// newline / CR / other control chars < 0x20 (except TAB) -> single space +// +// The output buffer is always NUL-terminated. Truncation is silent +// (mirrors strlcpy semantics used elsewhere in this file). Returns the +// number of bytes written, excluding the NUL terminator. +// + +static size_t // O - Bytes written +ppd_escape_string(char *out, // I - Output buffer + size_t out_size, // I - Size of output + // buffer + const char *in) // I - Input string +{ + size_t oi = 0; + const unsigned char *p; + + if (!out || out_size == 0) + return (0); + if (!in) + { + out[0] = '\0'; + return (0); + } + + for (p = (const unsigned char *)in; *p && oi + 2 < out_size; p++) + { + unsigned char c = *p; + + if (c == '\\' || c == '"') + { + if (oi + 3 >= out_size) + break; + out[oi++] = '\\'; + out[oi++] = c; + } + else if (c == '\n' || c == '\r' || (c < 0x20 && c != '\t')) + { + // Squash control chars that could break out of the PPD quoted + // string context. + out[oi++] = ' '; + } + else + { + out[oi++] = c; + } + } + out[oi] = '\0'; + return (oi); +} + + char * // O - PPD filename or NULL - // on error + // on error ppdCreatePPDFromIPP2(char *buffer, // I - Filename buffer size_t bufsize, // I - Size of filename // buffer @@ -411,12 +477,28 @@ ppdCreatePPDFromIPP2(char *buffer, // I - Filename buffer } } - cupsFilePrintf(fp, "*Manufacturer: \"%s\"\n", make); - cupsFilePrintf(fp, "*ModelName: \"%s %s\"\n", make, model); - cupsFilePrintf(fp, "*Product: \"(%s %s)\"\n", make, model); + // + // Emit the make/model strings into quoted PPD directives. + // + // `make` and `model` originate from the IPP printer-make-and-model + // attribute of the discovered printer (or the DNS-SD fallback), i.e. + // attacker-influenceable input on an LAN with cups-browsed and + // avahi-daemon running. Route them through ppd_escape_string() first + // so that a payload containing '\n', '"', or '\\' cannot break out of + // the quoted PPD string context and mint additional PPD directives + // in the generated file. See regression tests + // ppd/test_ppd_generator.c :: T08b / T08c. + // + char make_esc[512], model_esc[512]; + ppd_escape_string(make_esc, sizeof(make_esc), make); + ppd_escape_string(model_esc, sizeof(model_esc), model); + + cupsFilePrintf(fp, "*Manufacturer: \"%s\"\n", make_esc); + cupsFilePrintf(fp, "*ModelName: \"%s %s\"\n", make_esc, model_esc); + cupsFilePrintf(fp, "*Product: \"(%s %s)\"\n", make_esc, model_esc); cupsFilePrintf(fp, "*NickName: \"%s %s, %sdriverless, %s\"\n", - make, model, (is_fax ? "Fax, " : ""), VERSION); - cupsFilePrintf(fp, "*ShortNickName: \"%s %s\"\n", make, model); + make_esc, model_esc, (is_fax ? "Fax, " : ""), VERSION); + cupsFilePrintf(fp, "*ShortNickName: \"%s %s\"\n", make_esc, model_esc); // Which is the default output bin? if ((attr = ippFindAttribute(supported, "output-bin-default", diff --git a/ppd/test_ppd_generator.c b/ppd/test_ppd_generator.c index fcb6b3f6..10be2cf2 100644 --- a/ppd/test_ppd_generator.c +++ b/ppd/test_ppd_generator.c @@ -6,7 +6,7 @@ // Licensed under Apache License v2.0. See the file "LICENSE" for more // information. // -// Tests covered (45 assertions across 12 groups): +// Tests covered (48 assertions across 13 groups): // // Group 1 (T01-T05) NULL / argument guards and smoke test. // ppdCreatePPDFromIPP2 returns NULL with errno @@ -27,6 +27,14 @@ // make takes the "No separate model name" branch // at line 374 → model = "Printer". // +// Group 2b (T08b-T08d) PPD string-escape hardening. Regression tests +// for ppd_escape_string() applied to make/model +// before emission at ppd/ppd-generator.c:480-485. +// Verifies newline, double-quote, and backslash +// in printer-make-and-model are neutralized so +// no additional PPD directives can be minted in +// the generated file. +// // Group 3 (T09-T10) HP normalization. "Hewlett Packard " and // "Hewlett-Packard " (both 16-char prefixes, // line 357) are rewritten to "HP" + the @@ -355,6 +363,94 @@ main(void) ippDelete(resp); + // ========================================================================= + // Group 2b: PPD string-escape hardening (T08b - T08d) + // ========================================================================= + // + // Regression tests for ppd_escape_string() applied to make/model + // before emission at ppd/ppd-generator.c:480-485 (Manufacturer, + // ModelName, Product, NickName, ShortNickName). + // + // Motivation: IPP `printer-make-and-model` is a network-influenceable + // string (LAN mDNS-discovered printer + cups-browsed). Without escape, + // a make containing '\n', '"', or '\\' can close its own quoted PPD + // string and mint new PPD directives in the generated file. + // + // These tests do NOT assert that any specific downstream parser would + // then execute the injected directive; they lock in the emitter's + // escape discipline so future refactors do not silently reopen the + // gap regardless of parser semantics. + + // T08b — printer-make-and-model with an embedded newline. The '\n' + // must be squashed to a space (not preserved as a raw byte) + // so that the *Manufacturer / *ModelName / *NickName lines + // each stay as one PPD directive line, not two. + testBegin("printer-make-and-model with '\\n' emits single-line *Manufacturer"); + resp = ippNew(); + add_format(resp, "application/pdf"); + ippAddString(resp, IPP_TAG_PRINTER, IPP_TAG_TEXT, + "printer-make-and-model", NULL, + "Acme\n*FoomaticRIPCommandLine: \"pwn\""); + result = ppdCreatePPDFromIPP2(buffer, sizeof(buffer), resp, + NULL, "application/pdf", + 0, 0, NULL, NULL, NULL, NULL, NULL, 0); + ppd_text = result ? slurp_file(buffer) : NULL; + testEnd(ppd_text && + // The injected directive must NOT appear as a new PPD directive + // (start of a line with '*'). + strstr(ppd_text, + "\n*FoomaticRIPCommandLine: \"pwn\"") == NULL && + // The escaped form (payload folded onto the Manufacturer line + // with the newline replaced by a space) must appear. + strstr(ppd_text, + "*Manufacturer: \"Acme " + "*FoomaticRIPCommandLine:") != NULL); + if (result == buffer) unlink(buffer); + free(ppd_text); + ippDelete(resp); + + // T08c — printer-make-and-model with an embedded double-quote. + // The '"' must be backslash-escaped so the *Manufacturer value + // is not prematurely terminated. + testBegin("printer-make-and-model with '\"' emits backslash-escaped value"); + resp = ippNew(); + add_format(resp, "application/pdf"); + ippAddString(resp, IPP_TAG_PRINTER, IPP_TAG_TEXT, + "printer-make-and-model", NULL, + "Acme\" *Product: \"pwn"); + result = ppdCreatePPDFromIPP2(buffer, sizeof(buffer), resp, + NULL, "application/pdf", + 0, 0, NULL, NULL, NULL, NULL, NULL, 0); + ppd_text = result ? slurp_file(buffer) : NULL; + testEnd(ppd_text && + // The escaped form must be present (backslash-quote). + strstr(ppd_text, + "*Manufacturer: \"Acme\\\"") != NULL); + if (result == buffer) unlink(buffer); + free(ppd_text); + ippDelete(resp); + + // T08d — printer-make-and-model with an embedded backslash. + // A raw '\\' must be doubled so it does not consume the + // following character in later PPD parsers. + testBegin("printer-make-and-model with '\\\\' emits doubled backslash"); + resp = ippNew(); + add_format(resp, "application/pdf"); + ippAddString(resp, IPP_TAG_PRINTER, IPP_TAG_TEXT, + "printer-make-and-model", NULL, + "Acme\\Model"); + result = ppdCreatePPDFromIPP2(buffer, sizeof(buffer), resp, + NULL, "application/pdf", + 0, 0, NULL, NULL, NULL, NULL, NULL, 0); + ppd_text = result ? slurp_file(buffer) : NULL; + testEnd(ppd_text && + strstr(ppd_text, + "*Manufacturer: \"Acme\\\\Model\"") != NULL); + if (result == buffer) unlink(buffer); + free(ppd_text); + ippDelete(resp); + + // ========================================================================= // Group 3: HP normalization (T09-T10) // =========================================================================