fix: prototype pollution in mergeObj and URL scheme validation in Drafty entity props

src/drafty.js

@@ -286,6 +286,23 @@ function base64toDataUrl(b64, contentType) {
   return 'data:' + contentType + ';base64,' + b64;
 }
 
+// Allow only URLs with safe schemes to prevent javascript: and data: injection.
+// Returns null for URLs with disallowed schemes, the original value otherwise.
+function sanitizeUrl(url) {
+  if (!url || typeof url != 'string') {
+    return url;
+  }
+  // Relative URLs have no scheme and are safe.
+  if (!/^\s*([a-z][a-z0-9+.-]*:|\/\/)/im.test(url)) {
+    return url;
+  }
+  // Among absolute URLs allow only http, https, and ftp.
+  if (/^(https?|ftp):\/\//i.test(url)) {
+    return url;
+  }
+  return null;
+}
+
 // Helpers for converting Drafty to HTML.
 const DECORATORS = {
   // Visial styles
@@ -328,7 +345,7 @@ const DECORATORS = {
     close: _ => '</a>',
     props: (data) => {
       return data ? {
-        href: data.url,
+        href: sanitizeUrl(data.url),
         target: '_blank'
       } : null;
     },
@@ -366,7 +383,7 @@ const DECORATORS = {
         'data-act': data.act,
         'data-val': data.val,
         'data-name': data.name,
-        'data-ref': data.ref
+        'data-ref': sanitizeUrl(data.ref)
       } : null;
     },
   },
@@ -379,10 +396,11 @@ const DECORATORS = {
     close: _ => '</audio>',
     props: (data) => {
       if (!data) return null;
+      const safeRef = sanitizeUrl(data.ref);
       return {
         // Embedded data or external link.
-        src: data.ref || base64toObjectUrl(data.val, data.mime, Drafty.logger),
-        'data-preload': data.ref ? 'metadata' : 'auto',
+        src: safeRef || base64toObjectUrl(data.val, data.mime, Drafty.logger),
+        'data-preload': safeRef ? 'metadata' : 'auto',
         'data-duration': data.duration,
         'data-name': data.name,
         'data-size': data.val ? ((data.val.length * 0.75) | 0) : (data.size | 0),
@@ -410,7 +428,7 @@ const DECORATORS = {
       return {
         // Temporary preview, or permanent preview, or external link.
         src: base64toDataUrl(data._tempPreview, data.mime) ||
-          data.ref || base64toObjectUrl(data.val, data.mime, Drafty.logger),
+          sanitizeUrl(data.ref) || base64toObjectUrl(data.val, data.mime, Drafty.logger),
         title: data.name,
         alt: data.name,
         'data-width': data.width,
@@ -474,14 +492,16 @@ const DECORATORS = {
     close: _ => '',
     props: data => {
       if (!data) return null;
-      const poster = data.preref || base64toObjectUrl(data.preview, data.premime || 'image/jpeg', Drafty.logger);
+      const safePreref = sanitizeUrl(data.preref);
+      const safeRef = sanitizeUrl(data.ref);
+      const poster = safePreref || base64toObjectUrl(data.preview, data.premime || 'image/jpeg', Drafty.logger);
       return {
         // Embedded data or external link.
         src: poster,
-        'data-src': data.ref || base64toObjectUrl(data.val, data.mime, Drafty.logger),
+        'data-src': safeRef || base64toObjectUrl(data.val, data.mime, Drafty.logger),
         'data-width': data.width,
         'data-height': data.height,
-        'data-preload': data.ref ? 'metadata' : 'auto',
+        'data-preload': safeRef ? 'metadata' : 'auto',
         'data-preview': poster,
         'data-duration': data.duration | 0,
         'data-name': data.name,
@@ -1913,7 +1933,7 @@ Drafty.getDownloadUrl = function(entData) {
   if (!Drafty.isFormResponseType(entData.mime) && entData.val) {
     url = base64toObjectUrl(entData.val, entData.mime, Drafty.logger);
   } else if (typeof entData.ref == 'string') {
-    url = entData.ref;
+    url = sanitizeUrl(entData.ref);
   }
   return url;
 }

src/drafty.test.js

@@ -1598,3 +1598,33 @@ const quote_this = [
 test.each(quote_this)('Drafty.quote %j', (src, exp) => {
   expect(Drafty.quote("tinode-user", "usrbzV_721mIW0", src)).toEqual(exp);
 })
+
+// Tests for URL scheme validation via Drafty.attrValue (item 3: no URL scheme validation).
+test('Drafty.attrValue LN sanitizeUrl', () => {
+  // Safe schemes are passed through.
+  expect(Drafty.attrValue('LN', {url: 'https://example.com/path'})).toEqual({href: 'https://example.com/path', target: '_blank'});
+  expect(Drafty.attrValue('LN', {url: 'http://example.com'})).toEqual({href: 'http://example.com', target: '_blank'});
+  expect(Drafty.attrValue('LN', {url: 'ftp://files.example.com'})).toEqual({href: 'ftp://files.example.com', target: '_blank'});
+  // Relative URLs are safe.
+  expect(Drafty.attrValue('LN', {url: '/v0/file/s/abc.jpg'})).toEqual({href: '/v0/file/s/abc.jpg', target: '_blank'});
+  expect(Drafty.attrValue('LN', {url: 'relative/path.html'})).toEqual({href: 'relative/path.html', target: '_blank'});
+  // Unsafe schemes are blocked.
+  expect(Drafty.attrValue('LN', {url: "javascript:alert('XSS')"})).toEqual({href: null, target: '_blank'});
+  expect(Drafty.attrValue('LN', {url: 'data:text/html,<script>alert(1)</script>'})).toEqual({href: null, target: '_blank'});
+  expect(Drafty.attrValue('LN', {url: 'vbscript:msgbox(1)'})).toEqual({href: null, target: '_blank'});
+  // Protocol-relative URLs are also blocked (scheme is inherited from page context).
+  expect(Drafty.attrValue('LN', {url: '//evil.com/path'})).toEqual({href: null, target: '_blank'});
+});
+
+test('Drafty.attrValue BN sanitizeUrl', () => {
+  expect(Drafty.attrValue('BN', {act: 'url', ref: 'https://example.com', name: 'btn', val: 'v'}))
+    .toEqual({'data-act': 'url', 'data-val': 'v', 'data-name': 'btn', 'data-ref': 'https://example.com'});
+  expect(Drafty.attrValue('BN', {act: 'url', ref: "javascript:alert(1)", name: 'btn', val: 'v'}))
+    .toEqual({'data-act': 'url', 'data-val': 'v', 'data-name': 'btn', 'data-ref': null});
+});
+
+test('Drafty.getDownloadUrl sanitizeUrl', () => {
+  expect(Drafty.getDownloadUrl({mime: 'image/jpeg', ref: 'https://example.com/img.jpg'})).toBe('https://example.com/img.jpg');
+  expect(Drafty.getDownloadUrl({mime: 'image/jpeg', ref: "javascript:alert(1)"})).toBeNull();
+  expect(Drafty.getDownloadUrl({mime: 'image/jpeg', ref: '/v0/file/s/abc.jpg'})).toBe('/v0/file/s/abc.jpg');
+});

src/utils.js

@@ -90,16 +90,18 @@ export function mergeObj(dst, src) {
   }
 
   if (!dst || dst === DEL_CHAR) {
-    dst = src.constructor();
+    dst = {};
   }
 
-  for (let prop in src) {
-    if (src.hasOwnProperty(prop) && (prop != '_noForwarding')) {
-      try {
-        dst[prop] = mergeObj(dst[prop], src[prop]);
-      } catch (err) {
-        console.warn("Error merging property:", prop, err);
-      }
+  for (const prop of Object.keys(src)) {
+    // Skip prototype-polluting keys and internal forwarding flag.
+    if (prop === '__proto__' || prop === 'constructor' || prop === '_noForwarding') {
+      continue;
+    }
+    try {
+      dst[prop] = mergeObj(dst[prop], src[prop]);
+    } catch (err) {
+      console.warn("Error merging property:", prop, err);
     }
   }
   return dst;

src/utils.test.js

@@ -110,6 +110,14 @@ test('mergeObj', () => {
     a: 1,
     b: 2
   }, 1)).toEqual(1);
+  // Prototype pollution: __proto__ key must not pollute Object.prototype.
+  const polluted = mergeObj({}, JSON.parse('{"__proto__":{"injected":true}}'));
+  expect(({}).injected).toBeUndefined();
+  expect(polluted.injected).toBeUndefined();
+  // Prototype pollution: constructor key must not overwrite constructor.
+  const withCtor = mergeObj({}, JSON.parse('{"constructor":{"prototype":{"injected2":true}}}'));
+  expect(({}).injected2).toBeUndefined();
+  expect(typeof withCtor.constructor).toBe('function');
 });
 
 // Strips all values from an object of they evaluate to false or if their name starts with '_'.