4 ms·

Here's a link to the fix: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65954/diffs https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65954/...

tankenmate 5y ago
I get the sneaking suspicion that this is a case of Ruby's magic being slightly too magic; it's a problem I have tripped across in the past. Slightly tangentially reminds me of the "More Magic" switch of GLS fame.
- LewisVerstappen 5y ago
  >Slightly tangentially reminds me of the "More Magic" switch of GLS fame. Can you elaborate on what you're referencing? Tried googling but couldn't find anything.
  - robocat 5y ago
    http://www.catb.org/jargon/html/magic-story.html http://www.catb.org/jargon/html/magic-story.html
    - mdip 5y ago
      Thanks for the link -- you solved a nagging mystery for me[0]. In the '90s, I worked for a tech guy from Texas who referred to the clippers that we used to cut the ends of zip ties off as dikes. Of course, being a teenager, the term often came with a raised eyebrow from me since it was commonly used as a slur for lesbians. He was the only person I've encountered in my life who used that term in that manner and over the years I just assumed it was some bit of obscurity related to where he started working in systems. I guess not! [0] I'm sure the right Gooble query would have gotten me an answer but it teetered on the edge of "I don't care enough to bother" until the answer was presented.
- zorr 5y ago
  There is not much ruby magic in that code. This is just a naive use of data from another module. Somewhere in the Gitlab code base there is a MIME_TYPES map with common extensions as the map key. No idea what it is used for but that module is very likely the target of a recent security issue. The first fix to combat the "publicly unknown" vulnerability was to prevent usernames ending with any of the keys in the MIME_TYPES map using a simple "ends_with" strings check. Of course the map keys did not have periods so the ends_with would also match "Asimov" with the "mov" suffix. The second fix in this PR is to extend the ends_with check to add an extra dot. The actual vulnerability is still unknown but I suspect it's something like an intermediate component that performs special handling based on interpreting URLs and that could bypass security/ACL checks.
- jeltz 5y ago
  Look at the code instead of guessing. This bug could have happened in any language.
  - marcosdumay 5y ago
    There is no good reason to create that code, unless your dependencies have some problem. This does really look like a "too much magic" situation.
  - gkop 5y ago
    Curious, how much time have you spent with Ruby on Rails? (Which GitLab is based on) Rails has a special track record for convenient magic implicated in security vulnerabilities. Another commenter gave a good example theory implicating a convenient-but-questionable out of the box behavior of Rails: https://news.ycombinator.com/item?id=28537562 https://news.ycombinator.com/item?id=28537562 The good news is Rails has been slowly moving away from the magic over the years - it used to be a lot worse.

tankenmate 5y ago

I get the sneaking suspicion that this is a case of Ruby's magic being slightly too magic; it's a problem I have tripped across in the past. Slightly tangentially reminds me of the "More Magic" switch of GLS fame.

LewisVerstappen 5y ago
>Slightly tangentially reminds me of the "More Magic" switch of GLS fame. Can you elaborate on what you're referencing? Tried googling but couldn't find anything.
- robocat 5y ago
  http://www.catb.org/jargon/html/magic-story.html http://www.catb.org/jargon/html/magic-story.html
  - mdip 5y ago
    Thanks for the link -- you solved a nagging mystery for me[0]. In the '90s, I worked for a tech guy from Texas who referred to the clippers that we used to cut the ends of zip ties off as dikes. Of course, being a teenager, the term often came with a raised eyebrow from me since it was commonly used as a slur for lesbians. He was the only person I've encountered in my life who used that term in that manner and over the years I just assumed it was some bit of obscurity related to where he started working in systems. I guess not! [0] I'm sure the right Gooble query would have gotten me an answer but it teetered on the edge of "I don't care enough to bother" until the answer was presented.
zorr 5y ago
There is not much ruby magic in that code. This is just a naive use of data from another module. Somewhere in the Gitlab code base there is a MIME_TYPES map with common extensions as the map key. No idea what it is used for but that module is very likely the target of a recent security issue. The first fix to combat the "publicly unknown" vulnerability was to prevent usernames ending with any of the keys in the MIME_TYPES map using a simple "ends_with" strings check. Of course the map keys did not have periods so the ends_with would also match "Asimov" with the "mov" suffix. The second fix in this PR is to extend the ends_with check to add an extra dot. The actual vulnerability is still unknown but I suspect it's something like an intermediate component that performs special handling based on interpreting URLs and that could bypass security/ACL checks.
jeltz 5y ago
Look at the code instead of guessing. This bug could have happened in any language.
- marcosdumay 5y ago
  There is no good reason to create that code, unless your dependencies have some problem. This does really look like a "too much magic" situation.
- gkop 5y ago
  Curious, how much time have you spent with Ruby on Rails? (Which GitLab is based on) Rails has a special track record for convenient magic implicated in security vulnerabilities. Another commenter gave a good example theory implicating a convenient-but-questionable out of the box behavior of Rails: https://news.ycombinator.com/item?id=28537562 https://news.ycombinator.com/item?id=28537562 The good news is Rails has been slowly moving away from the magic over the years - it used to be a lot worse.