Add latest changes from gitlab-org/security/gitlab@14-3-stable-ee

7 files changed, 69 insertions, 10 deletions

diff --git a/app/assets/javascripts/lib/dompurify.js b/app/assets/javascripts/lib/dompurify.js
index d421d66981e..47ede8cb1bb 100644
--- a/app/assets/javascripts/lib/dompurify.js
+++ b/app/assets/javascripts/lib/dompurify.js
@@ -1,5 +1,5 @@
 import { sanitize as dompurifySanitize, addHook } from 'dompurify';
-import { getBaseURL, relativePathToAbsolute } from '~/lib/utils/url_utility';
+import { getNormalizedURL, getBaseURL, relativePathToAbsolute } from '~/lib/utils/url_utility';
 
 const defaultConfig = {
   // Safely allow SVG <use> tags
@@ -11,12 +11,14 @@ const defaultConfig = {
 
 // Only icons urls from `gon` are allowed
 const getAllowedIconUrls = (gon = window.gon) =>
-  [gon.sprite_file_icons, gon.sprite_icons].filter(Boolean);
+  [gon.sprite_file_icons, gon.sprite_icons]
+    .filter(Boolean)
+    .map((path) => relativePathToAbsolute(path, getBaseURL()));
 
-const isUrlAllowed = (url) => getAllowedIconUrls().some((allowedUrl) => url.startsWith(allowedUrl));
+const isUrlAllowed = (url) =>
+  getAllowedIconUrls().some((allowedUrl) => getNormalizedURL(url).startsWith(allowedUrl));
 
-const isHrefSafe = (url) =>
-  isUrlAllowed(url) || isUrlAllowed(relativePathToAbsolute(url, getBaseURL())) || url.match(/^#/);
+const isHrefSafe = (url) => url.match(/^#/) || isUrlAllowed(url);
 
 const removeUnsafeHref = (node, attr) => {
   if (!node.hasAttribute(attr)) {
@@ -36,13 +38,14 @@ const removeUnsafeHref = (node, attr) => {
  *   <use href="/assets/icons-xxx.svg#icon_name"></use>
  * </svg>
  *
+ * It validates both href & xlink:href attributes.
+ * Note that `xlink:href` is deprecated, but still in use
+ * https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href
+ *
  * @param {Object} node - Node to sanitize
  */
 const sanitizeSvgIcon = (node) => {
   removeUnsafeHref(node, 'href');
-
-  // Note: `xlink:href` is deprecated, but still in use
-  // https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href
   removeUnsafeHref(node, 'xlink:href');
 };
 
diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js
index bca0e45d98d..deedc210e5c 100644
--- a/app/assets/javascripts/lib/utils/url_utility.js
+++ b/app/assets/javascripts/lib/utils/url_utility.js
@@ -397,6 +397,24 @@ export function isSafeURL(url) {
   }
 }
 
+/**
+ * Returns a normalized url
+ *
+ * https://gitlab.com/foo/../baz => https://gitlab.com/baz
+ *
+ * @param {String} url - URL to be transformed
+ * @param {String?} baseUrl - current base URL
+ * @returns {String}
+ */
+export const getNormalizedURL = (url, baseUrl) => {
+  const base = baseUrl || getBaseURL();
+  try {
+    return new URL(url, base).href;
+  } catch (e) {
+    return '';
+  }
+};
+
 export function getWebSocketProtocol() {
   return window.location.protocol.replace('http', 'ws');
 }
diff --git a/spec/frontend/lib/dompurify_spec.js b/spec/frontend/lib/dompurify_spec.js
index 324441fa2c9..47a94a4dcde 100644
--- a/spec/frontend/lib/dompurify_spec.js
+++ b/spec/frontend/lib/dompurify_spec.js
@@ -22,12 +22,16 @@ const safeUrls = {
 const unsafeUrls = [
   '/an/evil/url',
   '../../../evil/url',
-  'https://evil.url/assets/icons-123a.svg',
+  'https://evil.url/assets/icons-123a.svg#test',
   'https://evil.url/assets/icons-456b.svg',
   `https://evil.url/${rootGon.sprite_icons}`,
   `https://evil.url/${rootGon.sprite_file_icons}`,
   `https://evil.url/${absoluteGon.sprite_icons}`,
   `https://evil.url/${absoluteGon.sprite_file_icons}`,
+  `${rootGon.sprite_icons}/../evil/path`,
+  `${rootGon.sprite_file_icons}/../../evil/path`,
+  `${absoluteGon.sprite_icons}/../evil/path`,
+  `${absoluteGon.sprite_file_icons}/../../https://evil.url`,
 ];
 
 const forbiddenDataAttrs = ['data-remote', 'data-url', 'data-type', 'data-method'];
diff --git a/spec/frontend/lib/utils/url_utility_spec.js b/spec/frontend/lib/utils/url_utility_spec.js
index 6f186ba3227..74855524809 100644
--- a/spec/frontend/lib/utils/url_utility_spec.js
+++ b/spec/frontend/lib/utils/url_utility_spec.js
@@ -607,6 +607,27 @@ describe('URL utility', () => {
     });
   });
 
+  describe('getNormalizedURL', () => {
+    it.each`
+      url                       | base                                 | result
+      ${'./foo'}                | ${''}                                | ${'http://test.host/foo'}
+      ${'../john.md'}           | ${''}                                | ${'http://test.host/john.md'}
+      ${'/images/img.png'}      | ${'https://gitlab.com'}              | ${'https://gitlab.com/images/img.png'}
+      ${'/images/../img.png'}   | ${'https://gitlab.com'}              | ${'https://gitlab.com/img.png'}
+      ${'/images/./img.png'}    | ${'https://gitlab.com'}              | ${'https://gitlab.com/images/img.png'}
+      ${'./images/img.png'}     | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/user/images/img.png'}
+      ${'../images/../img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/img.png'}
+      ${'/images/img.png'}      | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/images/img.png'}
+      ${'/images/../img.png'}   | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/img.png'}
+      ${'/images/./img.png'}    | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/images/img.png'}
+    `(
+      'converts url "$url" with base "$base" to normalized url => "expected"',
+      ({ url, base, result }) => {
+        expect(urlUtils.getNormalizedURL(url, base)).toBe(result);
+      },
+    );
+  });
+
   describe('getWebSocketProtocol', () => {
     it.each`
       protocol    | expectation
diff --git a/workhorse/internal/upload/exif/testdata/takes_lot_of_memory_to_decode.tiff b/workhorse/internal/upload/exif/testdata/takes_lot_of_memory_to_decode.tiff
new file mode 100644
index 00000000000..6935cb130db
--- /dev/null
+++ b/workhorse/internal/upload/exif/testdata/takes_lot_of_memory_to_decode.tiff
diff --git a/workhorse/internal/upload/rewrite.go b/workhorse/internal/upload/rewrite.go
index 79ebfe950c5..3dfab120188 100644
--- a/workhorse/internal/upload/rewrite.go
+++ b/workhorse/internal/upload/rewrite.go
@@ -233,7 +233,7 @@ func handleExifUpload(ctx context.Context, r io.Reader, filename string, imageTy
 }
 
 func isTIFF(r io.Reader) bool {
-	_, err := tiff.Decode(r)
+	_, err := tiff.DecodeConfig(r)
 	if err == nil {
 		return true
 	}
diff --git a/workhorse/internal/upload/rewrite_test.go b/workhorse/internal/upload/rewrite_test.go
index 6fc41c3fefd..e3f33a02489 100644
--- a/workhorse/internal/upload/rewrite_test.go
+++ b/workhorse/internal/upload/rewrite_test.go
@@ -2,6 +2,7 @@ package upload
 
 import (
 	"os"
+	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -29,6 +30,10 @@ func TestImageTypeRecongition(t *testing.T) {
 			filename: "exif/testdata/sample_exif_invalid.jpg",
 			isJPEG:   false,
 			isTIFF:   false,
+		}, {
+			filename: "exif/testdata/takes_lot_of_memory_to_decode.tiff", // File from https://gitlab.com/gitlab-org/gitlab/-/issues/341363
+			isJPEG:   false,
+			isTIFF:   true,
 		},
 	}
 
@@ -36,8 +41,16 @@ func TestImageTypeRecongition(t *testing.T) {
 		t.Run(test.filename, func(t *testing.T) {
 			input, err := os.Open(test.filename)
 			require.NoError(t, err)
+
+			var m runtime.MemStats
+			runtime.ReadMemStats(&m)
+			start := m.TotalAlloc
+
 			require.Equal(t, test.isJPEG, isJPEG(input))
 			require.Equal(t, test.isTIFF, isTIFF(input))
+
+			runtime.ReadMemStats(&m)
+			require.Less(t, m.TotalAlloc-start, uint64(50000), "must take reasonable amount of memory to recognise the type")
 		})
 	}
 }