Live data from Hacker News

Escaping user input is ridonkulously hard

codeofhonor.substack.com

71–80 of 88 posts

Re: Escaping user input is ridonkulously hard

#71

Earlier quoted context omitted.

There's just a whole library of CVEs for people who attempt to escape things being sent to SQL. Use parameterized queries already.

every one says "just use parameterized queries" but they don't handle arrays which makes the idea rather useless.

Use one that does? Or build it yourself?

    ARRAY[?, ?, ?, ?]

Re: Escaping user input is ridonkulously hard

#72
post #70
post #31

Earlier quoted context omitted.

>If you have some function that accepts it, blindly casts it to UTF-8 Unfortunately, if you interact with services you didn't write, you're usually back to getting "strings" of unknown encoding, and typically requirements that force some blind or semi-blind guessing.

Blind guessing is not related to the type system. Nobody has claimed type systems can solve that. What they can do is force you to guess, and make it clear where that is occurring. This, again, goes back to a very broken understanding of types systems that I often see, and once held myself. The claim of type systems is not that they magically go out into the world and fix the external world to be well-typed; the clai…

I agree with that if you qualify it with "sometimes". Strong types can force you to guess, sometimes. Other times, the data fits the type but isn't the type.

Re: Escaping user input is ridonkulously hard

#73

Earlier quoted context omitted.

> The point is that even with proper types, this is not easy to manage or fix. In practice, in a typed language, nothing like this ever occurs, because the rule is just: "use string for everything, except the edge". You're thinking of a type like: HtmlString >> In practice the type that is "passed around" is almost always just "string", and this is converted at the last moment to a single destination format, such as…

> In practice the type that is "passed around" is almost always just "string" That's what happens in practice, of course. The GP was proposing something else, and I was explaining how complicated that gets. > and this is converted at the last moment to a single destination format, such as HtmlString. I explained before why this doesn't work unless we're talking about the final destination of this string. Otherwise, i…

You're basically running around with your finger on the trigger and suggesting that everyone everywhere ought to wear ballistic armour to compensate.

This is how you put the safety on and return the gun into its holster:

    using System;
    using System.Text.Json;
    
    string maliciousInput = "{0} % $0 -- DROP TABLE \"USERS\"";
    
    // Always, always, ALWAYS use a proper serializer for assembling formats like JSON.
    // The malicious input can include actual JavaScript, and it'll be correctly encoded with 100% safety.
    string encoded = JsonSerializer.Serialize( new {
        context= "{0}",      // .NET format string placeholder
        input=maliciousInput
    });
    
    // This will just work, formatting placeholders are ignored if no parameters are specified
    Console.WriteLine(encoded);
    
    // A safe FormatException is thrown if you mis-use the string formmating code. 
    // No vulnerability other than DDoS.
    Console.WriteLine(encoded, "adfasfd");

Test here: https://dotnetfiddle.net/p8P1fO

Fundamentally, putting any format like JSON or any user-controlled input into the first parameter of sprintf or any similar function in any language is Wrong with a capital W. It ought to be picked up in code review.

Ideally, sprintf-like functions in strongly typed languages should use a special "FormatString" type instead of a plain string as the first input. This would automatically fix any such issues, but relying on this is still problematic. Naively printing potentially malicious input to places like the console is still quite dangerous, no matter how much you escape it! Logs can be captured into systems that then paste it directly into HTML. Similarly, console control codes can be used by attackers as a nuisance. Etc... Structured logging, along the lines of OpenTelemetry is safer.

See: https://owasp.org/www-community/attacks/Log_Injection

This is the safe equivalent of your second example. Both format strings and JSON are correctly handled:

    Console.WriteLine( "{0}", JsonSerializer.Serialize( new {
     context="{0}", // if sprintf/WriteLine is not misused delibaretely, this is safe!
     input="safe-looking\", \"bypassAuth\": \"true\"}" 
    }));
This outputs:

    {"context":"{0}","input":"safe-looking\u0022, \u0022bypassAuth\u0022: \u0022true\u0022}"}
Link: https://dotnetfiddle.net/Lm8jkR

Re: Escaping user input is ridonkulously hard

#74
post #7

An alternate view: “string” is not a granular enough type, just like “bitfield” is not a type. Firstly, a string could be raw unknown bytes, verified UTF-8, or UCS-2 (or even UTF-16 or UCS-4), and you absolutely need to know which it is. But let’s assume that you’ve been a diligent programmer and filtered all that at the edges, and now have a sequence of Unicode code points (or possibly graphemes). You still need to…

> a string could be raw unknown bytes, verified UTF-8, or UCS-2 (or even UTF-16 or UCS-4)

Agreed. My future perfect programming language has the predefined types 'ascii', 'utf-8', 'url', 'base64', etc. for misc kinds of character sequences.

Just like how raw bits are different from numerals: short vs byte, word vs int, 64-bits vs double, etc.

(Any one have a better naming system for 8, 16, 32, and 64 bit chunks of raw data? 'byte', 'word', 'doubleword', 'quadword'?)

Per this "ridonkulously hard" OC article, I'll also ponder predefined types for raw 'html5', 'json', etc (as in unparsed, char sequence vs DOM).

--

> Perl was early with its concept of “tainted” strings.

Not being a Perl dev, I'm unfamiliar with "taint". Quickly found articles like this: https://www.geeksforgeeks.org/perl-taint-method/

In my future perfect language, char seqs cannot be cast. They must be converted. Basically syntactic sugar for Java-style char encoding infrastructure.

I have assumed that disallowing casting was sufficient. But now I'll have to ponder "taint" too. From the hip, I really like the notion of tracking the provenance of data, a la defensive programming.

Great idea. Thanks.

Re: Escaping user input is ridonkulously hard

#75

Earlier quoted context omitted.

> In practice the type that is "passed around" is almost always just "string" That's what happens in practice, of course. The GP was proposing something else, and I was explaining how complicated that gets. > and this is converted at the last moment to a single destination format, such as HtmlString. I explained before why this doesn't work unless we're talking about the final destination of this string. Otherwise, i…

You're basically running around with your finger on the trigger and suggesting that everyone everywhere ought to wear ballistic armour to compensate. This is how you put the safety on and return the gun into its holster: using System; using System.Text.Json; string maliciousInput = "{0} % $0 -- DROP TABLE \"USERS\""; // Always, always, ALWAYS use a proper serializer for assembling formats like JSON. // The malicious…

Neither of your examples actually replaces the {0} in "context" with "some context", so they are not achieving the desired output (which would have been `{"context": "some context", "input": "{0} % $0 -- DROP TABLE \\\"USERS\\\""}`). They are equivalent to my "safe but wrong" examples. The point here was that you may want to "template-ize" the produced JSON for whatever reason.

Also, DoS is a legitimate concern, and of course using a safe language makes other consequences less dire. I wasn't using sprintf() in my example for nothing.

> Naively printing potentially malicious input to places like the console is still quite dangerous, no matter how much you escape it! Logs can be captured into systems that then paste it directly into HTML.

This is exactly my point: when you include untrusted input into another string, even if you escape the untrusted input correctly for the desired format of that string, the entire output is now untrusted, and generally can't be further processed safely. Yours is a perfect example: you can escape the user input to make sure it is formatted safely, but you can't at this point tell in what other ways it should be escaped for other systems that may process it (for example, even printing to the actual console like this may be unsafe, as the user input may include terminal control characters).

Even this problem is still simple if we can assume that, say, anything printed to the console that later needs to be displayed in HTML should be considered an HTML string - it just becomes a simple responsibility of the log collector to properly escape the log lines as HTML content.

The problem is much harder if the intention is to actually control the HTML output through log lines (say, adding new-lines through br in your log statements, or emphasis or whatever). If that is a necessary component of your system, you need to re-architect this so that log lines themselves are no longer simple strings, but are structured so that any user-controlled input is kept separate from the trusted application-control formatting

Say, instead of logging

  error: user entered "some|||stringalert('pwned')" which is not a valid number
starting over
you would log

  error: user entered %s, which is not a valid number
starting over ||| some\|\|\|stringalert('pwned')`
and the log collector would need to know to recombine it into the original string, escaping the untrusted part as needed, before outputting it to HTML as

  error: user entered "some|||string<script>alert('pwned')</script>",  which is not a valid number
starting over
Edit: interestingly, I had to use instead of a normal opening script tag, as HN would give me a TLS error if the comment contains the normal opening script tag...

Wonder if there is some input sanitization going on here as well.

Re: Escaping user input is ridonkulously hard

#76

Earlier quoted context omitted.

You're basically running around with your finger on the trigger and suggesting that everyone everywhere ought to wear ballistic armour to compensate. This is how you put the safety on and return the gun into its holster: using System; using System.Text.Json; string maliciousInput = "{0} % $0 -- DROP TABLE \"USERS\""; // Always, always, ALWAYS use a proper serializer for assembling formats like JSON. // The malicious…

Neither of your examples actually replaces the {0} in "context" with "some context", so they are not achieving the desired output (which would have been `{"context": "some context", "input": "{0} % $0 -- DROP TABLE \\\"USERS\\\""}`). They are equivalent to my "safe but wrong" examples. The point here was that you may want to "template-ize" the produced JSON for whatever reason. Also, DoS is a legitimate concern, and…

Formatting a string twice is what lead to the Log4j security vulnerability. It had a macro language that allowed user-controlled input to have macros expanded in an unexpected place. Essentially the macro input itself could contain macros.

Your example where you use a sprintf format-string placeholder inside an incomplete JSON snippet ought never be used! Ever.

It's not needed in practice. Construct the object graph and insert the parameters there (unescaped!) and then serialize the whole thing.

E.g.:

    JsonConverter.Serialize( new {
        // Use a static format string! Never let users control this...
        context = string.Format( "{0:N1}", userControlledMaliciosInput ),
        alsoThis = "... json snippet...",
    });
This is fine.

But as I was saying, the "even better" solution is to not serialize this into JSON and then "work with the string representation". The use of JSON[1] should be a detail transparent to 99% of the application. You should be able -- safely -- to switch it out for XML, gRPC, Cap'n Proto, or whatever.

Going back to my logging example, it ought not to matter what wire format something like OpenTelemetry uses. You should be able to use "rich" object graphs in logging calls, and then let the library figure it out. E.g.:

    Log.Information( "user submitted a form", new {
        context = string.Format( "{0:N1}", userControlledMaliciosInput ),
        alsoThis = "... json snippet...",
    });
Ideally, everything should treat this as the native "object" graph whenever the developer interacts with it. Only the "edges", such as RPC serialization or deserialization needs to deal with encoding, at which point it'll need to use exactly one escaping/encoding format and not have worry about nested encodings at all.

[1] A mistake in the design of JSON is that it appears to be "simple", so beginner programmers think they understand it and can work with it "directly" using string manipulation. This is ill-defined at the best of times, and downright unsafe in surprisingly common scenarios: http://seriot.ch/projects/parsing_json.html

Re: Escaping user input is ridonkulously hard

#77

Earlier quoted context omitted.

Neither of your examples actually replaces the {0} in "context" with "some context", so they are not achieving the desired output (which would have been `{"context": "some context", "input": "{0} % $0 -- DROP TABLE \\\"USERS\\\""}`). They are equivalent to my "safe but wrong" examples. The point here was that you may want to "template-ize" the produced JSON for whatever reason. Also, DoS is a legitimate concern, and…

Formatting a string twice is what lead to the Log4j security vulnerability. It had a macro language that allowed user-controlled input to have macros expanded in an unexpected place. Essentially the macro input itself could contain macros. Your example where you use a sprintf format-string placeholder inside an incomplete JSON snippet ought never be used! Ever. It's not needed in practice. Construct the object graph…

We're stuck discussing JSON and other data transfer encodings, which is partly my fault as I brought it up, but there are far more scenarios for using combined text encodings.

It is very common to have templating languages which include their own syntax + the syntax of a target output language (e.g. Markdown supports HTML snippets that should get output to the final HTML as is; C macros support C code snippets, and C itself supports Assembler snippets that should end up in the final binary etc). When generating/processing the mixed format from your own code, you may often hit the problems above.

Even for JSON, there are legitimate reasons for processing stored JSON documents as text, or at least situations where it seems a safe enough approach - because people tend to forget that a string representation of a JSON document that has user-controlled input should be itself considered untrusted user input in its entirety, at least unless it is parsed by a JSON parser.

Additionally, data often has to be stored to unstructured storage (e.g. disk) between the moment you receive untrusted user input and the moment you output the final format to the user - again, doing the easy thing of storing in the intermediate format with the first level of escaping of untrusted input is extremely tempting, and the alternative is significantly more difficult.

Re: Escaping user input is ridonkulously hard

#78
post #7

An alternate view: “string” is not a granular enough type, just like “bitfield” is not a type. Firstly, a string could be raw unknown bytes, verified UTF-8, or UCS-2 (or even UTF-16 or UCS-4), and you absolutely need to know which it is. But let’s assume that you’ve been a diligent programmer and filtered all that at the edges, and now have a sequence of Unicode code points (or possibly graphemes). You still need to…

> Firstly, a string could be raw unknown bytes, verified UTF-8, or UCS-2 (or even UTF-16 or UCS-4), and you absolutely need to know which it is. This is a language defect. If your language was invented in the 1960s it's an understandable defect, but it's still a defect. I do not want to write computer software with strings in a language that doesn't even have an actual string type rather than "Eh, maybe this is a str…

The way Rust does it is IMO interesting. There is e.g. an OsStr for strings that e.g. describe filenames in an directory listing, because these could actually be invalid UTF-8 but your program might still need to be able to handle them.

So when you wanna convert that OsStr to a String you are forced to handle this in one way or another. This is less comfortable, but describes the underlying systems more accurately.

Re: Escaping user input is ridonkulously hard

#79

Earlier quoted context omitted.

Formatting a string twice is what lead to the Log4j security vulnerability. It had a macro language that allowed user-controlled input to have macros expanded in an unexpected place. Essentially the macro input itself could contain macros. Your example where you use a sprintf format-string placeholder inside an incomplete JSON snippet ought never be used! Ever. It's not needed in practice. Construct the object graph…

We're stuck discussing JSON and other data transfer encodings, which is partly my fault as I brought it up, but there are far more scenarios for using combined text encodings. It is very common to have templating languages which include their own syntax + the syntax of a target output language (e.g. Markdown supports HTML snippets that should get output to the final HTML as is; C macros support C code snippets, and C…

All of the use-cases you listed I would flag in a code-review as fundamentally misguided.

If you have formats "A" and "B" with serialization functions A() and B() that take document object models as inputs (not strings!), then nesting them is valid, but a bit of a code smell.

What you're saying is that there are scenarios where A() and B() take strings and return strings, and those strings can have control codes that "mean something" for A and/or B.

That's inherently bad and dangerous, and was the direct cause of one of the WORST vulnerabilities in history. Literally as bad as anything ever out there.

You're saying "maximum bad" is a good idea sometimes. This is like making the argument that a little nuclear war is acceptable on occasion.

> there are legitimate reasons for processing stored JSON documents as text

No, there isn't. Stop. Never do this. Ever.

Don't parse HTML or XML with Regex either. It leads to m̷͉̈a̴̳̚d̶̟̐n̴̩̓e̷̘̿s̴̤͆s̵͉͗: https://stackoverflow.com/questions/1732348/regex-match-open...

Re: Escaping user input is ridonkulously hard

#80
post #65
post #59

Earlier quoted context omitted.

Strong typing never “solved” anything, except that is forces you to see the problem and solve it yourself, explicitly, instead of relying on weak typing to fudge the types for you.

Maybe. Ambiguity means some random bags of bytes pass as more than one type.

And the "Any" type covereth a multitude of sins.
Post reply on HN