8 ms·

Looks like the fix restricted the check to "usernames that end with dot and MIME type" Still, what is the attack vector?

by ncann 5y ago

boomskats 5y ago
This doesn't look like a security issue, unless I'm missing something.
- pavon 5y ago
  Indeed the MR template does not have the security box checked.
  - kinix 5y ago
    My guess is that the username is used in a url somewhere? So browsers might try and interpret it as a file
    - numlock86 5y ago
      That's what actual MIME types are for. You can serve a .jpg as a .txt file and still have the browser display it correctly as an image if you use image/jpeg. Same for the other way around or any other combination of file and MIME types.
      - kinix 5y ago
        You're not wrong, but I bet there are lots of things that will assume based on url (rightly or wrongly). e.g. I've seen a lot of nginx reverse proxy setups implement different caching policies based on url suffix (again rightly or wrongly). Maybe just playing it safe?
        numlock86 5y ago
        Yeah, if you for example use the 1001th hip electron based browser (with some hand-crafted curl workers and custom fetchers with an express based proxy-backend included in the background mangled together with some weird mix of 20% javascript and 80% typescript and a combination of at least 400 different npm packages over a commit history of two weekends) to browse the web ... of course, that assumption might break. :^)
      - jasonjayr 5y ago
        Do you remember when MSIE would ignore the MIME type the server sent, and sniffed the contents of the beginning of the file downloaded and overrode what the server sent? Good times ..... (rocking in corner ....)
- LewisVerstappen 5y ago
  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.
- nine_k 5y ago
  It could maybe potentially be exploited in a very interestingly crafted email, where there's link to download something (e.g. the source tarball, or a build artifact) with an URL containing the username, or being otherwise close by, so that the downloaded file would be interpreted differently. But I'm not creative enough at this hour to suggest a working exploit.
  - foota 5y ago
    I've played with this before. A correctly implemented mail library should handle e.g., subject lines that contain SMTP control characters. I developed a lengthy repro for an email parsing issue in an ancient version of some java email library that contained a truely horrendous parser, only to find out that the library had been updated internally recently :-)
  - a-dub 5y ago
    MIME types are used all over the place: 1) web servers, browsers, proxies 2) graphical os shells 3) email every file a webserver returns has a mime type in the header, and that is how the browser knows how to present it.
    - dolmen 5y ago
      The ticket here reveals some blacklisting of known file extensions (attached to a know list of MIME type), not MIME types directly.
    - mmis1000 5y ago
      Even webserver returns a mime, browser may sometimes still ignore it and guess it themselves, because server may be wrong. Unless the server ask them `absolutely don't do it` via the header. a.k.a. `X-Content-Type-Options: nosniff` But even there is a header for that, there will still be situations that clients don't care and guess it themselves (probably because software is too old or other reason).
      - amenghra 5y ago
        Correct. You want to set Content-Type and X-Content-Type-Options. The story is that in some cases, Internet Explorer could be tricked to ignore the Content-Type header. Instead of fixing the core bug, Microsoft decided to add a header (presumably, they didn’t want to break existing websites). A decade later, people discovered two ways to bypass X-Content-Type-Options. This time, Microsoft fixed the core issue instead of adding X-Content-Type-Options-For-Reals.
- paxys 5y ago
  Definitely a security issue. - The merge request which originally added this check is inaccessible (https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1450 https://gitlab.com/gitlab-org/security/gitlab/-/merge_reques...) - In the issue comments the Gitlab employee says "Sorry, I cannot go into details right now. I will link the issue here once it goes public, is it ok?"
walty8 5y ago
But the in the screen capture of article, the user name is actually 'issac.asimov', i.e. the mime type does not immediately follow the dot.
- ajkjk 5y ago
  That was before the fix.
- nobody9999 5y ago
  >But the in the screen capture of article, the user name is actually 'issac.asimov', i.e. the mime type does not immediately follow the dot. A variation on the Scunthorpe Problem[0] then, eh? [0] https://en.wikipedia.org/wiki/Scunthorpe_problem https://en.wikipedia.org/wiki/Scunthorpe_problem
  - whizzter 5y ago
    Somebody probably put in a regexp with .mov$ , however for regexps the dot (.) matches everything (and $ matches end) so the i in asimov is eaten regardless and then the rest of the match succeeds.
    - chippiewill 5y ago
      You can see the fix they made in the linked MR. It wasn't a regex, they just did a generic "ends with" check.
  - iechoz6H 5y ago
    Perhaps the sub-clause is redundant there? 'The problem was named after an incident in 1996 in which AOL's profanity filter prevented residents of the town of Scunthorpe, Lincolnshire, England, from creating accounts with AOL, because the town's name contains the substring "cunt".'
    - nobody9999 5y ago
      >'The problem was named after an incident in 1996 in which AOL's profanity filter prevented residents of the town of Scunthorpe, Lincolnshire, England, from creating accounts with AOL, because the town's name contains the substring "cunt".' Right. Regardless of the specific pattern matching function, in both cases, the results were both incorrect and unwanted. Which is why I consider this instance to be a variation on the same issue.
anentropic 5y ago
Probably: some web frameworks do content negotiation by appending a content type like .json to the end of the url Not sure if it's an attack vector per se, or just that the behaviour is incompatible with allowing usernames containing . and then having urls where the username is the last segment of the url seems like a badly designed url scheme :)
- jfrunyon 5y ago
  > some web frameworks do content negotiation by appending a content type like .json to the end of the url This has always disturbed me, considering that HTTP has had content negotiation for ... oh, basically its entire history [https://www.w3.org/Protocols/HTTP/1.0/spec.html#Accept https://www.w3.org/Protocols/HTTP/1.0/spec.html#Accept].
  - Macha 5y ago
    For non-programmatic usage and verification, the extensions can be easier? On a similar topic HIBP allowed people to request versioning via a custom HTTP Header, a Accept Content-Type, or a version segment in the URL path and approximately everyone went with option 3.
    - jfrunyon 5y ago
      "For non-programmatic usage and verification," it should be served in a human-readable manner, typically including things like formatting, and maybe even syntax coloring if you're feeling nice. Take my `Accept: text/html` and give me my HTML-ified JSON, dangit! ;)
      - Macha 5y ago
        I've actually seen APIs with "pretty=true" or "indent=4" type query parameters to emit formatted JSON for people before, though it doesn't go as far as HTML with syntax colouring.
- gkop 5y ago

Looks like the fix restricted the check to "usernames that end with dot and MIME type" Still, what is the attack vector?

by ncann 5y ago

Looks like the fix restricted the check to "usernames that end with dot and MIME type" Still, what is the attack vector?

boomskats 5y ago

This doesn't look like a security issue, unless I'm missing something.

pavon 5y ago
Indeed the MR template does not have the security box checked.
- kinix 5y ago
  My guess is that the username is used in a url somewhere? So browsers might try and interpret it as a file
  - numlock86 5y ago
    That's what actual MIME types are for. You can serve a .jpg as a .txt file and still have the browser display it correctly as an image if you use image/jpeg. Same for the other way around or any other combination of file and MIME types.
    - kinix 5y ago
      You're not wrong, but I bet there are lots of things that will assume based on url (rightly or wrongly). e.g. I've seen a lot of nginx reverse proxy setups implement different caching policies based on url suffix (again rightly or wrongly). Maybe just playing it safe?
      - numlock86 5y ago
        Yeah, if you for example use the 1001th hip electron based browser (with some hand-crafted curl workers and custom fetchers with an express based proxy-backend included in the background mangled together with some weird mix of 20% javascript and 80% typescript and a combination of at least 400 different npm packages over a commit history of two weekends) to browse the web ... of course, that assumption might break. :^)
    - jasonjayr 5y ago
      Do you remember when MSIE would ignore the MIME type the server sent, and sniffed the contents of the beginning of the file downloaded and overrode what the server sent? Good times ..... (rocking in corner ....)
LewisVerstappen 5y ago
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.
nine_k 5y ago
It could maybe potentially be exploited in a very interestingly crafted email, where there's link to download something (e.g. the source tarball, or a build artifact) with an URL containing the username, or being otherwise close by, so that the downloaded file would be interpreted differently. But I'm not creative enough at this hour to suggest a working exploit.
- foota 5y ago
  I've played with this before. A correctly implemented mail library should handle e.g., subject lines that contain SMTP control characters. I developed a lengthy repro for an email parsing issue in an ancient version of some java email library that contained a truely horrendous parser, only to find out that the library had been updated internally recently :-)
- a-dub 5y ago
  MIME types are used all over the place: 1) web servers, browsers, proxies 2) graphical os shells 3) email every file a webserver returns has a mime type in the header, and that is how the browser knows how to present it.
  - dolmen 5y ago
    The ticket here reveals some blacklisting of known file extensions (attached to a know list of MIME type), not MIME types directly.
  - mmis1000 5y ago
    Even webserver returns a mime, browser may sometimes still ignore it and guess it themselves, because server may be wrong. Unless the server ask them `absolutely don't do it` via the header. a.k.a. `X-Content-Type-Options: nosniff` But even there is a header for that, there will still be situations that clients don't care and guess it themselves (probably because software is too old or other reason).
    - amenghra 5y ago
      Correct. You want to set Content-Type and X-Content-Type-Options. The story is that in some cases, Internet Explorer could be tricked to ignore the Content-Type header. Instead of fixing the core bug, Microsoft decided to add a header (presumably, they didn’t want to break existing websites). A decade later, people discovered two ways to bypass X-Content-Type-Options. This time, Microsoft fixed the core issue instead of adding X-Content-Type-Options-For-Reals.
paxys 5y ago
Definitely a security issue. - The merge request which originally added this check is inaccessible (https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1450 https://gitlab.com/gitlab-org/security/gitlab/-/merge_reques...) - In the issue comments the Gitlab employee says "Sorry, I cannot go into details right now. I will link the issue here once it goes public, is it ok?"

walty8 5y ago

But the in the screen capture of article, the user name is actually 'issac.asimov', i.e. the mime type does not immediately follow the dot.

ajkjk 5y ago
That was before the fix.
nobody9999 5y ago
>But the in the screen capture of article, the user name is actually 'issac.asimov', i.e. the mime type does not immediately follow the dot. A variation on the Scunthorpe Problem[0] then, eh? [0] https://en.wikipedia.org/wiki/Scunthorpe_problem https://en.wikipedia.org/wiki/Scunthorpe_problem
- whizzter 5y ago
  Somebody probably put in a regexp with .mov$ , however for regexps the dot (.) matches everything (and $ matches end) so the i in asimov is eaten regardless and then the rest of the match succeeds.
  - chippiewill 5y ago
    You can see the fix they made in the linked MR. It wasn't a regex, they just did a generic "ends with" check.
- iechoz6H 5y ago
  Perhaps the sub-clause is redundant there? 'The problem was named after an incident in 1996 in which AOL's profanity filter prevented residents of the town of Scunthorpe, Lincolnshire, England, from creating accounts with AOL, because the town's name contains the substring "cunt".'
  - nobody9999 5y ago
    >'The problem was named after an incident in 1996 in which AOL's profanity filter prevented residents of the town of Scunthorpe, Lincolnshire, England, from creating accounts with AOL, because the town's name contains the substring "cunt".' Right. Regardless of the specific pattern matching function, in both cases, the results were both incorrect and unwanted. Which is why I consider this instance to be a variation on the same issue.

anentropic 5y ago

Probably: some web frameworks do content negotiation by appending a content type like .json to the end of the url Not sure if it's an attack vector per se, or just that the behaviour is incompatible with allowing usernames containing . and then having urls where the username is the last segment of the url seems like a badly designed url scheme :)

jfrunyon 5y ago
> some web frameworks do content negotiation by appending a content type like .json to the end of the url This has always disturbed me, considering that HTTP has had content negotiation for ... oh, basically its entire history [https://www.w3.org/Protocols/HTTP/1.0/spec.html#Accept https://www.w3.org/Protocols/HTTP/1.0/spec.html#Accept].
- Macha 5y ago
  For non-programmatic usage and verification, the extensions can be easier? On a similar topic HIBP allowed people to request versioning via a custom HTTP Header, a Accept Content-Type, or a version segment in the URL path and approximately everyone went with option 3.
  - jfrunyon 5y ago
    "For non-programmatic usage and verification," it should be served in a human-readable manner, typically including things like formatting, and maybe even syntax coloring if you're feeling nice. Take my `Accept: text/html` and give me my HTML-ified JSON, dangit! ;)
    - Macha 5y ago
      I've actually seen APIs with "pretty=true" or "indent=4" type query parameters to emit formatted JSON for people before, though it doesn't go as far as HTML with syntax colouring.
gkop 5y ago