Hacker News Reader

Comment by lolinder | Hacker News Reader

> Normally, id should be a number. However when id is {"to_s": {"bytesize": 2, "to_s": "1234REDIS_COMMANDS" }}, we can inject additional redis commands by using bytesize to limit the previous command when it is constructed....

This class of bug cannot happen in a statically-typed language. When I deserialize something in Java, I know what it turned into. If it wasn't what I expected, I get a stacktrace, not an RCE vulnerability.

I could write an interpreter to do weird stuff with my custom class and that could lead to an RCE, or I might have a library that does so. But barring severe malfeasance in a trusted library (like log4j) I'll never think I have a plain number and actually have untrusted code.

View on HN

Java had a long list of this class of vulnerability, because it lets you build frameworks that lookup class by name.

It's no longer common to do that in Java, because after the CVEs were reported, framework authors changed the API. This process also happened in ruby. Not familiar enough with other ecosystems to be sure but I'd bet money it's hit a fair few of them.

I'd file that under "weird stuff you can do with the deserialized object", and static types certainly don't protect you from that (edit: or from dangerous serializer libraries).

This bug isn't in that category, though: this happened because GitLab assumed they had a specific data type but didn't guarantee that this was always true. The build_command method, in turn, does something different depending on the type of data passed into it.

This line in the Ruby code:

    id = id_for_already_imported_cache(object)

Would translate to this in the Java code:

    Object id = idForAlreadyImportedCache(object)

Which would immediately set off alarm bells in the mind of the author that they should probably double-check they've got the right type.

There's a similar set of problems with .NET and BinaryFormatter and a few other things.

MS now have a security guide on using them: https://learn.microsoft.com/en-us/dotnet/standard/serializat...

I love and prefer Java & Static languages too, however saying it cannot happen in a static language is a bit of stretch. The recent Log4j vulnerability and countless Jackson vulnerabilities are examples of how a simple deserialiation can gone wrong.

Jackson can accept "@class": "com.package.something.Class" which might be class that executes commands on constructor, there is a huge denylist to keep you safe when you use that feature. Almost similar to what Gitlab have here.

Log4J was not a case of "simple" deserialization. The feature that caused the vulnerability was designed for remote execution of code. For this reason I called it out explicitly as an exception.

Vulnerabilities in the serialization library are a separate class of bug. This gitlab bug wasn't a serialization vulnerability, it was a flawed assumption about the structure of the data. The serializer did its job just fine.

In the case of Jackson's @class tagging, the key thing is that Java will still force you to think about the type. If you expect the `id` field to be an integer, you'll have to eventually cast Object to the class that you think you have, and that cast will fail if you got something else instead. There's no way for a redis command to make its way into an int field.

Have you heard about this: https://www.zdnet.com/article/equifax-confirms-apache-struts...

Pretty much everyone's private credit data in the US were leaked because of code written in... Java. Should we still go on into this discussion about how a statically-typed language saves you from whatever, or we can bury this finally?

This is what I said:

> This class of bug cannot happen in a statically-typed language.

I never claimed statically typed languages prevented all RCE bugs, I claimed they prevent RCE bugs that result from making bad assumptions about the shape of your data, which is what happened here with gitlab.

The RCE you linked to was the result of including an interpreter for a scripting language and incorrectly executing it during error handling. This is exactly the kind of malfeasance I identified in my original comment as potentially breaking the safety static typing can bring. Even this could have been prevented by better use of the type system: the OGNL interpreter shouldn't be able to accept just any String, it should require wrapping the String in an OGNLScript class first. As long as library code doesn't construct that class, this would force all layers to keep track of whether a given string is actually safe to interpret. The user must at some point opt in.

Back to the gitlab exploit: It's a fact that a statically-typed language does save you from bugs whose explanation is "id is supposed to be a number, but..." We can quibble about whether it's worth the hassle to have that guarantee, but this particular bug would not have been written in Java.

Reply on news.ycombinator.com