[HN Gopher] RCE via GitHub import
___________________________________________________________________
RCE via GitHub import
Author : louislang
Score : 95 points
Date : 2022-10-10 19:33 UTC (3 hours ago)
(HTM) web link (gitlab.com)
(TXT) w3m dump (gitlab.com)
| staticassertion wrote:
| > Sawyer is a crazy class that converts a hash to an object whose
| methods are based on the hash's key:
|
| People keep adding hidden interpreters and 'exec' commands in
| their projects where no one expects them to be. As far as I can
| tell the library is just supposed to make HTTP requests, it's
| just a fancy client library?
|
| This needs to be the kind of thing that developers are trained
| on, like "don't hash passwords" and "don't build up SQL strings
| from untrusted input". "Don't build your own 'exec' and hide it
| in your library, don't reinvent object serialization".
|
| There is almost always a better way to solve a problem than by
| transmitting arbitrary code. _Sometimes there isn 't_ but it's
| rare, and in those cases it's very explicit ie: "I'm sending this
| database a query, I expect it to execute it".
| rtev wrote:
| What's wrong with hashing passwords? Hash with a slow algorithm
| is the best method, as far as I know
| tialaramex wrote:
| Avoid passwords. "A secret is something you tell one other
| person, so I'm telling you". If possible adjust APIs to not
| rely on knowledge of secrets for their functioning, and then
| this entire problem evaporates. e.g. WebAuthn.
|
| If it's crucial that a human memorable secret (a password) is
| used, choose an asymmetrical Password Authenticated Key
| Exchange in which it's possible for the relying party to
| learn a value with which they can _confirm_ that the other
| party knows the password, but never learn what that password
| is. This is really difficult to do properly, OPAQUE is the
| current recommendation of the IETF for this purpose.
|
| Only fall back to hashing passwords because you're obliged to
| for legacy reasons.
| FreakLegion wrote:
| PAKEs bring little to the table when looked at in context
| of an actual threat model[1], meanwhile they add a fair
| degree of complexity and their implementations are much
| less battle-tested. Using them correctly won't really make
| anything more secure, but using them incorrectly might blow
| everything up.
|
| Don't avoid passwords with WebAuthn, either. "Fingerprints
| are usernames, not passwords".
|
| 1. https://palant.info/2018/10/25/should-your-next-web-
| based-lo...
| roblabla wrote:
| As one random point: If you just hash the password, you're
| vulnerable to rainbow table attacks. So you want to salt the
| password, at the very least.
|
| But really, what you want to do is use a framework developed
| by domain experts that deals with all that mess for you.
| Because there's a lot of surprising complexity to storing
| password hashes securely. So it's better to use a well-vetted
| library that has eyeballs and mindshare checking that it is
| correct.
| junon wrote:
| Rainbow table attacks are significantly harder with
| properly hashed passwords, e.g. with bcrypt.
| SahAssar wrote:
| I think all bcrypt implementations implement salting per
| default. Same for any modern password hashing
| implementation.
| lyu07282 wrote:
| I think that's what they are saying, bcrypt is secure
| because it uses a salt and multiple rounds of hashing.
| nine_k wrote:
| It's an easy slippery slope.
|
| We need flexibility because we can't predict all future needs,
| hence an interpreter.
|
| An interpreter can at worst produce a denial of service. But we
| also want it to access our data to be useful. Giving access to
| exact data items it may require is hard or impossible (see
| flexibility), so we give it a lump of access. Hello
| exfiltration.
|
| But crafting and supporting a limited interpreter is a chore.
| Why can't we use the perfectly good interpreter which runs our
| software? It's almost an RCE!
|
| For the win, we should note that input sanitation is hard and
| costs us CPU, and skip it. Now finally we made enough to have
| our system pwned.
| anamexis wrote:
| The RCE here doesn't come from Sawyer `exec`ing anything.
| Sawyer builds objects from a hashmap, where the hashmap's
| properties can be accessed as method calls, recursively.
|
| The vulnerability here arises from the fact that you can
| override built-in methods like `to_s`, which in combination
| with the way that the Redis gem builds raw commands, can be
| used to send arbitrary commands to Redis.
| mananaysiempre wrote:
| ... And the fact that Ruby doesn't really have fields, only
| methods. For example, the Python equivalent to this (morally,
| (obj := object()).__dict__.update(untrusted data)) would not
| be vulnerable. Which is not a point against Ruby, only
| against using this specific technique in Ruby.
| dwohnitmok wrote:
| Yeah... although monkey-patching, as this bug demonstrates,
| can get uncomfortably close to `exec`. I have a strong
| distaste for monkey-patching as a result since I'm only
| confident in its security when it's used purely statically
| (i.e. no user input can change how the monkey patching
| happens), but then that kind of robs the entire point of
| monkey-patching if you remove its dynamism.
| lolinder wrote:
| > 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.
| danielheath wrote:
| 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.
| lolinder wrote:
| I'd file that under "weird stuff you can do with the
| deserialized object", and static types certainly don't
| protect you from that.
|
| 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.
| paranoidrobot wrote:
| 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...
| paulgb wrote:
| > Sawyer is a crazy class that converts a hash to an object whose
| methods are based on the hash's key:
|
| For those not familiar with Ruby terminology, "hash" in this
| context means what other languages would call a hash map,
| associative array, or dictionary. At first I thought it was
| referring to the hash of a value, and couldn't grok it.
| btown wrote:
| For those on the Python side of things,
| https://box.readthedocs.io/en/development/ does something
| similar and is a great utility to reach for when you're
| handling trusted JSON-like data and need an API-like interface.
|
| (The Django templating language does this too, albeit in a more
| magical way: https://docs.djangoproject.com/en/4.1/ref/template
| s/language...)
|
| Just... make sure you're aware of the issue that the OP's RCE
| brings up!
| dwohnitmok wrote:
| Kudos on the comment log in demonstrating the transparency of how
| this was dealt with in the intervening month.
| whimsicalism wrote:
| It is interesting how far this discloser chose to go when
| investigating this vulnerability. They reproduced on their local
| Gitlab install, then proceeded to exploit the gitlab.com install
| itself and break one of their own gitlab repos.
|
| I am curious what the norms are here.
___________________________________________________________________
(page generated 2022-10-10 23:00 UTC)