diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-09-02 15:41:15 +0300 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-09-02 15:41:15 +0300 |
commit | 73c9795fd3ccf4b05cb52c052a0d90460a33eb9e (patch) | |
tree | 165dd227b103c0f7256881142767c78bab2effd5 | |
parent | ffa009f1d830f7ce710e89c5a8d9870fff42e252 (diff) | |
parent | e93a1b40496de4b3bc323b73dad4feafeb8c682b (diff) |
Merge remote-tracking branch 'dev/13-3-stable' into 13-3-stable
-rw-r--r-- | CHANGELOG.md | 7 | ||||
-rw-r--r-- | VERSION | 2 | ||||
-rw-r--r-- | internal/service/wiki/find_page_test.go | 71 | ||||
-rw-r--r-- | ruby/lib/gitlab/gollum.rb | 27 | ||||
-rw-r--r-- | ruby/proto/gitaly/version.rb | 2 | ||||
-rw-r--r-- | ruby/spec/lib/gitlab/gollum_spec.rb | 32 |
6 files changed, 137 insertions, 4 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 87df4624c..f1c8fa01b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Gitaly changelog +## 13.3.3 (2020-09-02) + +### Security (1 change) + +- Don't expand filesystem paths of wiki pages. + + ## 13.3.2 (2020-08-28) ### Fixed (1 change) @@ -1 +1 @@ -13.3.2
\ No newline at end of file +13.3.3
\ No newline at end of file diff --git a/internal/service/wiki/find_page_test.go b/internal/service/wiki/find_page_test.go index 4d5ab7249..1591ffb53 100644 --- a/internal/service/wiki/find_page_test.go +++ b/internal/service/wiki/find_page_test.go @@ -25,12 +25,19 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) { page3Name := "Installing/Step 133-c" page4Name := "Encoding is fun" page5Name := "Empty file" + page6Name := "~/Tilde in directory" + page7Name := "~Tilde in filename" + page8Name := "~!/Tilde with invalid user" + page1Commit := createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page1Name}) createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page2Name}) createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page3Name}) createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page4Name, content: []byte("f\xFCr")}) - page5Commit := createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page5Name, forceContentEmpty: true}) - latestCommit := page5Commit + createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page5Name, forceContentEmpty: true}) + createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page6Name}) + createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page7Name}) + page8Commit := createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page8Name}) + latestCommit := page8Commit testCases := []struct { desc string @@ -158,6 +165,66 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) { }, expectedContent: nil, }, + { + desc: "title for file with tilde in directory", + request: &gitalypb.WikiFindPageRequest{ + Repository: wikiRepo, + Title: []byte(page6Name), + }, + expectedPage: &gitalypb.WikiPage{ + Version: &gitalypb.WikiPageVersion{ + Commit: latestCommit, + Format: "markdown", + }, + Title: []byte("Tilde in directory"), + Format: "markdown", + UrlPath: "~/Tilde-in-directory", + Path: []byte("~/Tilde-in-directory.md"), + Name: []byte("Tilde in directory"), + Historical: false, + }, + expectedContent: mockPageContent, + }, + { + desc: "title for file with tilde in filename", + request: &gitalypb.WikiFindPageRequest{ + Repository: wikiRepo, + Title: []byte(page7Name), + }, + expectedPage: &gitalypb.WikiPage{ + Version: &gitalypb.WikiPageVersion{ + Commit: latestCommit, + Format: "markdown", + }, + Title: []byte(page7Name), + Format: "markdown", + UrlPath: "~Tilde-in-filename", + Path: []byte("~Tilde-in-filename.md"), + Name: []byte(page7Name), + Historical: false, + }, + expectedContent: mockPageContent, + }, + { + desc: "title for file with tilde and invalid user", + request: &gitalypb.WikiFindPageRequest{ + Repository: wikiRepo, + Title: []byte(page8Name), + }, + expectedPage: &gitalypb.WikiPage{ + Version: &gitalypb.WikiPageVersion{ + Commit: latestCommit, + Format: "markdown", + }, + Title: []byte("Tilde with invalid user"), + Format: "markdown", + UrlPath: "~!/Tilde-with-invalid-user", + Path: []byte("~!/Tilde-with-invalid-user.md"), + Name: []byte("Tilde with invalid user"), + Historical: false, + }, + expectedContent: mockPageContent, + }, } for _, testCase := range testCases { diff --git a/ruby/lib/gitlab/gollum.rb b/ruby/lib/gitlab/gollum.rb index 4bf08411f..cd9016dd8 100644 --- a/ruby/lib/gitlab/gollum.rb +++ b/ruby/lib/gitlab/gollum.rb @@ -17,4 +17,31 @@ module Gollum Gitlab::EncodingHelper.encode!(data) end end + + # Override BlobEntry.normalize_dir to remove the call to File.expand_path, + # which also expands `~` and `~username` paths, and can raise exceptions for + # invalid users. + # + # We don't need to worry about symlinks or Windows paths, we only need to + # normalize the slashes in the path, and return an empty string for toplevel + # paths. + class BlobEntry + def self.normalize_dir(dir) + # Return empty string for nil and paths that point to the toplevel + # ('.', '/', '..' etc.) + return '' if !dir || dir =~ %r{\A[\./]*\z} + + # Normalize the path: + # - Add exactly one leading slash + # - Remove trailing slashes + # - Remove repeated slashes + dir.sub(%r{ + \A + /* # leading slashes + (?<path>.*?) # the actual path + /* # trailing slashes + \z + }x, '/\k<path>').gsub(%r{//+}, '/') + end + end end diff --git a/ruby/proto/gitaly/version.rb b/ruby/proto/gitaly/version.rb index 04e552cf1..e2b2ba047 100644 --- a/ruby/proto/gitaly/version.rb +++ b/ruby/proto/gitaly/version.rb @@ -2,5 +2,5 @@ # (https://gitlab.com/gitlab-org/release-tools/), and should not be # modified. module Gitaly - VERSION = '13.3.2' + VERSION = '13.3.3' end diff --git a/ruby/spec/lib/gitlab/gollum_spec.rb b/ruby/spec/lib/gitlab/gollum_spec.rb new file mode 100644 index 000000000..0ee994c3d --- /dev/null +++ b/ruby/spec/lib/gitlab/gollum_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gollum do + describe Gollum::BlobEntry do + describe '.normalize_dir' do + it 'returns the path with normalized slashes' do + expect(described_class.normalize_dir('foo/bar')).to eq('/foo/bar') + expect(described_class.normalize_dir('/foo/bar')).to eq('/foo/bar') + expect(described_class.normalize_dir('/foo/bar/')).to eq('/foo/bar') + expect(described_class.normalize_dir('//foo//bar//')).to eq('/foo/bar') + end + + it 'returns an empty string for toplevel paths' do + expect(described_class.normalize_dir(nil)).to eq('') + expect(described_class.normalize_dir('')).to eq('') + expect(described_class.normalize_dir('.')).to eq('') + expect(described_class.normalize_dir('..')).to eq('') + expect(described_class.normalize_dir('/')).to eq('') + expect(described_class.normalize_dir('//')).to eq('') + expect(described_class.normalize_dir(' ')).to eq('/ ') + expect(described_class.normalize_dir("\t")).to eq("/\t") + end + + it 'does not expand tilde characters' do + expect(described_class.normalize_dir('~/foo')).to eq('/~/foo') + expect(described_class.normalize_dir('~root/foo')).to eq('/~root/foo') + expect(described_class.normalize_dir('~!/foo')).to eq('/~!/foo') + expect(described_class.normalize_dir('foo/~')).to eq('/foo/~') + end + end + end +end |