Sanitising job name and title #4072

Closed
michaelrsweet opened this Issue May 9, 2012 · 9 comments

Comments

Projects
None yet
1 participant
Collaborator

michaelrsweet commented May 9, 2012

Version: 1.5.2
CUPS.org User: ef

Some of our printer's network interfaces (SEH for Kyocera) seem to lock up if the job name contains a Ctrl-S character (I assume some left-over from X-ON/X-OFF communication with an LCD board or such). Unfortunately, this is not a purely academic issue since Browser printouts of Wikipedia pages regularly contain that character.

As SEH seems not fix that, I'm running with something like the attached patch (which simply replaces control characters with periods) for some years now. I'm not sure whether this patch is of value to someone else or worth being included.

Collaborator

michaelrsweet commented May 9, 2012

CUPS.org User: mike

Hmm, none of these places seems correct. We should reject invalidate job-name values in job submissions so that the backends never see a job-name with control characters in it.

Collaborator

michaelrsweet commented May 10, 2012

CUPS.org User: ef

Is there a definition of what characters are valid in an IPP Job Name?

Rejecting jobs with control characters in the job name would mean a regression to our users. They can hardly do anything about the Browser putting in the page title there.
Or do you intend to make lpr -T/lp -t sanitise its argument then?

Collaborator

michaelrsweet commented May 10, 2012

CUPS.org User: mike

RFC 2911 defines the valid characters for a "name' attribute, basically any character in the given character set. Supposedly there are some restrictions on control characters in RFC 3629 and 2045 (indirectly referenced by 2911) but I haven't found them yet.

I will be proposing some changes for this in the next draft of IPP Everywhere; whether these become implementation guidelines and security considerations or normative requirements is unclear, but we will be saying something about it.

IMHO, the correct place for validating the job-name is in cupsd when we receive it from a client/application (in create-job and print-job/uri operations). There we can either filter the value or reject it and substitute a placeholder.

Collaborator

michaelrsweet commented Sep 15, 2012

CUPS.org User: mike

Fixed in Subversion repository.

Collaborator

michaelrsweet commented Sep 16, 2012

CUPS.org User: ef

Ups, wouldn't this fail for almost any valid UTF-8 sequence due to nameptr getting out of sync with the characters?
I.e. on C3 9F 20, the C3 will pass but the 9F will fail.
Or am I getting something completely wrong?

I can also not read from RCF 5198 that TAB characters should be allowed.

Collaborator

michaelrsweet commented Sep 16, 2012

CUPS.org User: mike

OK, pulling back to fix that part of it...

Collaborator

michaelrsweet commented Sep 16, 2012

CUPS.org User: mike

OK, one more try, attached.

RFC 5198 talks about limiting which control characters are allowed for Network Unicode. IPP Everywhere talks about limiting the control characters to TAB for name and nameWithLanguage values.

Collaborator

michaelrsweet commented Sep 16, 2012

"str4072.patch":

Index: scheduler/ipp.c

--- scheduler/ipp.c (revision 10596)
+++ scheduler/ipp.c (working copy)
@@ -10943,7 +10943,8 @@
http_status_t status; /* Policy status /
ipp_attribute_t *attr, /
Current attribute /
*auth_info; /
auth-info attribute */

  • ipp_attribute_t format; / Document-format attribute */

  • ipp_attribute_t format, / Document-format attribute */

  •       _name;      /_ Job-name attribute _/
    

    cups_ptype_t dtype; /_ Destination type (printer/class) /
    char super[MIME_MAX_SUPER],
    /
    Supertype of file */
    @@ -10970,7 +10971,7 @@
    )
    {
    send_ipp_status(con, IPP_ATTRIBUTES,

  •                  _("Unsupported compression \"%s\"."),
    
  •                  _("Unsupported 'compression' value \"%s\"."),
              attr->values[0].string.text);
    

    ippAddString(con->response, IPP_TAG_UNSUPPORTED_GROUP, IPP_TAG_KEYWORD,
    "compression", NULL, attr->values[0].string.text);
    @@ -10988,7 +10989,8 @@
    if (sscanf(format->values[0].string.text, "%15[^/]/%31[^;]",
    super, type) != 2)
    {

  •  send_ipp_status(con, IPP_BAD_REQUEST, _("Bad document-format \"%s\"."),
    
  •  send_ipp_status(con, IPP_BAD_REQUEST,
    
  •                  _("Bad 'document-format' value \"%s\"."),
          format->values[0].string.text);
    

    return;
    }
    @@ -10999,7 +11001,7 @@
    cupsdLogMessage(CUPSD_LOG_INFO,
    "Hint: Do you have the raw file printing rules enabled?");
    send_ipp_status(con, IPP_DOCUMENT_FORMAT,

  •                  _("Unsupported document-format \"%s\"."),
    
  •                  _("Unsupported 'document-format' value \"%s\"."),
          format->values[0].string.text);
    

    ippAddString(con->response, IPP_TAG_UNSUPPORTED_GROUP, IPP_TAG_MIMETYPE,
    "document-format", NULL, format->values[0].string.text);
    @@ -11008,6 +11010,74 @@
    }

    /*

  • * Is the job-name valid?

  • */

  • if ((name = ippFindAttribute(con->request, "job-name", IPP_TAG_ZERO)) != NULL)
  • {
  • int bad_name = 0; /* Is the job-name value bad? */
  • if ((name->value_tag != IPP_TAG_NAME && name->value_tag != IPP_TAG_NAMELANG) ||
  •    name->num_values != 1)
    
  • {
  •  bad_name = 1;
    
  • }
  • else
  • {
  • /*
    
  •  \* Validate that job-name conforms to RFC 5198 (Network Unicode) and
    
  •  \* IPP Everywhere requirements for "name" values...
    
  •  */
    
  •  const unsigned char _nameptr;    /_ Pointer into "job-name" attribute */
    
  •  for (nameptr = (unsigned char *)name->values[0].string.text;
    
  •       *nameptr;
    
  •       nameptr ++)
    
  •  {
    
  •    if (*nameptr < ' ' && *nameptr != '\t')
    
  •      break;
    
  •    else if (*nameptr == 0x7f)
    
  •      break;
    
  •    else if ((*nameptr & 0xe0) == 0xc0 &&
    
  •             (nameptr[1] & 0xc0) != 0x80)
    
  •      break;
    
  •    else if ((*nameptr & 0xf0) == 0xe0 &&
    
  •             ((nameptr[1] & 0xc0) != 0x80 ||
    
  •              (nameptr[2] & 0xc0) != 0x80))
    
  •      break;
    
  •    else if ((*nameptr & 0xf8) == 0xf0 &&
    
  •             ((nameptr[1] & 0xc0) != 0x80 ||
    
  •              (nameptr[2] & 0xc0) != 0x80 ||
    
  •              (nameptr[3] & 0xc0) != 0x80))
    
  •      break;
    
  •    else if (*nameptr & 0x80)
    
  •      break;
    
  •  }
    
  •  if (*nameptr)
    
  •    bad_name = 1;
    
  • }
  • if (bad_name)
  • {
  •  if (StrictConformance)
    
  •  {
    
  • send_ipp_status(con, IPP_ATTRIBUTES,
  •               _("Unsupported 'job-name' value."));
    
  • ippCopyAttribute(con->response, name, 0);
  • return;
  •  }
    
  •  else
    
  •  {
    
  •    cupsdLogMessage(CUPSD_LOG_WARN,
    
  •                    "Unsupported 'job-name' value, deleting from request.");
    
  •    ippDeleteAttribute(con->request, name);
    
  •  }
    
  • }
  • }
  • /*
    • Is the destination valid?
      */
Collaborator

michaelrsweet commented Sep 16, 2012

"str4072p2.patch":

Index: scheduler/ipp.c

--- scheduler/ipp.c (revision 10606)
+++ scheduler/ipp.c (working copy)
@@ -11039,18 +11039,30 @@
break;
else if (*nameptr == 0x7f)
break;

  •    else if ((*nameptr & 0xe0) == 0xc0 &&
    
  •             (nameptr[1] & 0xc0) != 0x80)
    
  •      break;
    
  •    else if ((*nameptr & 0xf0) == 0xe0 &&
    
  •             ((nameptr[1] & 0xc0) != 0x80 ||
    
  •              (nameptr[2] & 0xc0) != 0x80))
    
  •      break;
    
  •    else if ((*nameptr & 0xf8) == 0xf0 &&
    
  •             ((nameptr[1] & 0xc0) != 0x80 ||
    
  •              (nameptr[2] & 0xc0) != 0x80 ||
    
  •              (nameptr[3] & 0xc0) != 0x80))
    
  •      break;
    
  •    else if ((*nameptr & 0xe0) == 0xc0)
    
  •    {
    
  •      if ((nameptr[1] & 0xc0) != 0x80)
    
  •        break;
    
  •      nameptr ++;
    
  •    }
    
  •    else if ((*nameptr & 0xf0) == 0xe0)
    
  •    {
    
  •      if ((nameptr[1] & 0xc0) != 0x80 ||
    
  •          (nameptr[2] & 0xc0) != 0x80)
    
  •   break;
    
  • nameptr += 2;
    
  • }
  •    else if ((*nameptr & 0xf8) == 0xf0)
    
  •    {
    
  •      if ((nameptr[1] & 0xc0) != 0x80 ||
    
  •     (nameptr[2] & 0xc0) != 0x80 ||
    
  •     (nameptr[3] & 0xc0) != 0x80)
    
  •   break;
    
  • nameptr += 3;
    
  • }
    else if (*nameptr & 0x80)
    break;
    }

michaelrsweet added this to the Stable milestone Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment