[Top] [All Lists]

Notify options and URI encoding

2007-08-21 06:00:45

Hello notifiers,

I re-read notify-08, and it jumped out at me that :options takes a
stringlist argument! I had it in mind that it was a single string, so
both :options and the method itself were equally good places for encoded
parameters. I had a significant concern about the interpretation of
something like:

    notify "method:whatever&subject=foo ${evilvar}";

where ${evilvar} might come directly from the message and might contain
nefarious data. That concern has been reflected in the draft with the
new :encodeurl variables parameter. But it's sort of a hack because
we're looking at encoding in the first place. 

I think that encoded parameters on the method name should be prohibited
outright. All ad-hoc parameters should arrive via :options. That's what
it's there for, right?

    notify :options "subject=Hello this is the Subject"

is much better than

    notify "mailto:foo(_at_)bar&subject=Hello%20this%20is%20the%20Subject";

Example 3 is particularly bad, starting with this variable:

    set "notif_method"

I think this should be re-written to show that :options is a string list
and to reflect that encoding is done by the implementation:

    set "notif_method" "xmpp:tim(_at_)example(_dot_)com?message"
    set "notif_subject" "SIEVE"
    set "notif_body" "You've got mail"

    ... tests in the example that override the defaults ...

    notify :options [ "subject=${notif_subject}", "body=${notif_body}" ]

Suggested new text in the description of :options, Section 3.5:

   Values may be URI-encoded and may be expanded from variables.
   Therefore, notification methods MUST NOT perform full URI-decoding
   of each option string, lest untrusted data contained in the variable
   become indistinguishable from user-specified data.

Security Considerations, this is from -08:

   Implementations that construct URIs internally from various notify
   parameters MUST make sure that all components of such URIs are
   properly percent-encoded (see [URI]).  In particular this applies to
   values of the :from and the :message tagged arguments and may apply
   to the :options values.

I think this is good, and we can drop Section 6, Modifier encodeurl,
too, as :encodeurl is not needed if the process for assembling a URI is
something like this (psuedo-perl):

    sub make_notify_uri( method, from, importance, options )
        return error unless sanity_check( method )
        uri = concat( method, "&from=", urlencode( from ) )
        uri = concat( uri, "&importance=", urlencode( importance ) )
        for each options
            next unless (key, value) =~ /([:alnum:])=(.*)/
            uri = concat( uri, "&", key, "=", urlencode( value ) )

The two important factors here are disallowing encoded methods and being
careful in how the option strings are split. I think that covers all of
the potential encoding holes for passing data through.


<Prev in Thread] Current Thread [Next in Thread>