4 ms·

GitLab team member here. I'd like to add some additional context to my previous comments [1][2]. Due to a security concern in which a profile containing a file

GitLab team member here. I'd like to add some additional context to my previous comments [1][2]. Due to a security concern in which a profile containing a file extension would not load [3], we do not allow usernames that end with file extensions (ex: .mov). As noted by many folks here, these are associated with a MIME type but are not MIME types themselves. It is not related to preventing an injection or any such attack vector. The error message for this check incorrectly included MIME type rather than file extension. This has been updated [4]. Additionally, there was an issue with how the actual check as it did not include the leading dot. The leading dot was added to the check in a subsequent MR [5]. Thanks for all the feedback. 1 - https://news.ycombinator.com/item?id=28535739 https://news.ycombinator.com/item?id=28535739 2 - https://news.ycombinator.com/item?id=28538166 https://news.ycombinator.com/item?id=28538166 3 - https://gitlab.com/gitlab-org/gitlab/-/issues/26295 https://gitlab.com/gitlab-org/gitlab/-/issues/26295 4 - https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70374/diffs https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70374/... 5 - https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65954 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65954

Arnavion 5y ago
>Due to a security concern in which a profile containing a file extension would not load [3], we do not allow usernames that end with file extensions (ex: .mov). Why did you not fix your routing engine to not consider file extensions where the username / group name should go?
- bogwog 5y ago
  gitlab profile URLs are `gitlab.com/<username>`, so a user with the name "dashboard.html" would have the URL `gitlab.com/dashboard.html`, which obviously conflicts with the existing dashboard.html file. Besides blacklisting certain usernames or breaking a bunch links to profiles, how would you fix that? EDIT: IIRC, github has the same issue, but they have profiles as lower priority. So if your username conflicts with an existing URL, your profile page doesn't work.
  - smallbizdev420 5y ago
    Isn't the problem arising because GitLabs files are in the global namespace? If the user is the namespace for all their files, and GitLab files were under a Gitlab user, this wouldn't be a problem. Under the current implementation, every time you add a file, you have to make sure its name doesn't conflict with an existing profile. And a username has to avoid conflict with all present and future filenames. Mutual pain doesn't seem like a good way forward.
  - Arnavion 5y ago
    For dashboard.html, sure. Fixing that requires making a breaking change to URLs. My comment, and the issue that was submitted here, is about *.mov
  - boleary-gl 5y ago
    GitHub doesn't allow a "." or really an special characters besides "-" and "_" in usernames
  - jhugo 5y ago
    In what situation would someone be requesting `https://gitlab.com/dashboard.html https://gitlab.com/dashboard.html`? When I go there, I get the exact same page as I get at `https://gitlab.com/ https://gitlab.com/`, why was it necessary to support both URLs? Now they're stuck with it of course, if anyone actually uses /dashboard.html, but surely they could just special-case filenames that actually exist, just like they presumably special-case URLs they use like /help already. It doesn't seem necessary to blanket-ban anything with a file extension.
    - remram 5y ago
      Or gitlab.com/dashboard. GitLab, GitHub, etc already have a need to reserve specific usernames (like `org`, `settings`, `projects`, `new`, `explore`, `marketplace`, `help`, ...). Since you already have to blocklist specific names not containing extensions, I really don't see how banning extensions help them. Hopefully we'll know more once their security ticket becomes public.
  - 5y ago
symlinkk 5y ago
Band aids on top of band aids. Respect for being honest and open about it though
- stephenr 5y ago
  I prefer the term “lipstick on a pig”.

Arnavion 5y ago

>Due to a security concern in which a profile containing a file extension would not load [3], we do not allow usernames that end with file extensions (ex: .mov). Why did you not fix your routing engine to not consider file extensions where the username / group name should go?

bogwog 5y ago
gitlab profile URLs are `gitlab.com/<username>`, so a user with the name "dashboard.html" would have the URL `gitlab.com/dashboard.html`, which obviously conflicts with the existing dashboard.html file. Besides blacklisting certain usernames or breaking a bunch links to profiles, how would you fix that? EDIT: IIRC, github has the same issue, but they have profiles as lower priority. So if your username conflicts with an existing URL, your profile page doesn't work.
- smallbizdev420 5y ago
  Isn't the problem arising because GitLabs files are in the global namespace? If the user is the namespace for all their files, and GitLab files were under a Gitlab user, this wouldn't be a problem. Under the current implementation, every time you add a file, you have to make sure its name doesn't conflict with an existing profile. And a username has to avoid conflict with all present and future filenames. Mutual pain doesn't seem like a good way forward.
- Arnavion 5y ago
  For dashboard.html, sure. Fixing that requires making a breaking change to URLs. My comment, and the issue that was submitted here, is about *.mov
- boleary-gl 5y ago
  GitHub doesn't allow a "." or really an special characters besides "-" and "_" in usernames
- jhugo 5y ago
  In what situation would someone be requesting `https://gitlab.com/dashboard.html https://gitlab.com/dashboard.html`? When I go there, I get the exact same page as I get at `https://gitlab.com/ https://gitlab.com/`, why was it necessary to support both URLs? Now they're stuck with it of course, if anyone actually uses /dashboard.html, but surely they could just special-case filenames that actually exist, just like they presumably special-case URLs they use like /help already. It doesn't seem necessary to blanket-ban anything with a file extension.
  - remram 5y ago
    Or gitlab.com/dashboard. GitLab, GitHub, etc already have a need to reserve specific usernames (like `org`, `settings`, `projects`, `new`, `explore`, `marketplace`, `help`, ...). Since you already have to blocklist specific names not containing extensions, I really don't see how banning extensions help them. Hopefully we'll know more once their security ticket becomes public.
- 5y ago

symlinkk 5y ago

Band aids on top of band aids. Respect for being honest and open about it though

stephenr 5y ago
I prefer the term “lipstick on a pig”.