Просмотр исходного кода

Merge pull request #7292 from weseek/fix/url-validation

fix: Invalid URL in markdown breaks browser
Yuki Takei 3 лет назад
Родитель
Сommit
54d30791dd

+ 10 - 2
packages/app/src/components/PageEditor/DrawioModal.tsx

@@ -9,14 +9,15 @@ import {
   ModalBody,
 } from 'reactstrap';
 
-
 import { getDiagramsNetLangCode } from '~/client/util/locale-utils';
 import { useDrawioUri } from '~/stores/context';
 import { useDrawioModal } from '~/stores/modal';
 import { usePersonalSettings } from '~/stores/personal-settings';
+import loggerFactory from '~/utils/logger';
 
 import { DrawioCommunicationHelper } from './DrawioCommunicationHelper';
 
+const logger = loggerFactory('growi:components:DrawioModal');
 
 const headerColor = '#334455';
 const fontFamily = "Lato, -apple-system, BlinkMacSystemFont, 'Hiragino Kaku Gothic ProN', Meiryo, sans-serif";
@@ -52,7 +53,14 @@ export const DrawioModal = (): JSX.Element => {
       return undefined;
     }
 
-    const url = new URL(drawioUri);
+    let url;
+    try {
+      url = new URL(drawioUri);
+    }
+    catch (err) {
+      logger.debug(err);
+      return undefined;
+    }
 
     // refs: https://desk.draw.io/support/solutions/articles/16000042546-what-url-parameters-are-supported-
     url.searchParams.append('spin', '1');

+ 22 - 17
packages/app/src/components/PageEditor/LinkEditModal.jsx

@@ -18,15 +18,19 @@ import validator from 'validator';
 import Linker from '~/client/models/Linker';
 import { apiv3Get } from '~/client/util/apiv3-client';
 import { useCurrentPagePath } from '~/stores/page';
+import loggerFactory from '~/utils/logger';
 
 import PagePreviewIcon from '../Icons/PagePreviewIcon';
 import SearchTypeahead from '../SearchTypeahead';
 
 import Preview from './Preview';
 
+
 import styles from './LinkEditPreview.module.scss';
 
 
+const logger = loggerFactory('growi:components:LinkEditModal');
+
 class LinkEditModal extends React.PureComponent {
 
   constructor(props) {
@@ -96,17 +100,24 @@ class LinkEditModal extends React.PureComponent {
     // create url from link, add dummy origin if link is not valid url.
     // ex-1. link = 'https://growi.org/' -> url = 'https://growi.org/' (case-1,4)
     // ex-2. link = 'hoge' -> url = 'http://example.com/hoge' (case-2,3,5)
-    const url = new URL(link, 'http://example.com');
-    const isUrl = url.origin !== 'http://example.com';
-
+    let isFqcn = false;
     let isUseRelativePath = false;
-    let reshapedLink = link;
+    let url;
+    try {
+      const url = new URL(link, 'http://example.com');
+      isFqcn = url.origin !== 'http://example.com';
+    }
+    catch (err) {
+      logger.debug(err);
+    }
 
-    // if case-1, reshapedLink becomes page path
-    reshapedLink = this.convertUrlToPathIfPageUrl(reshapedLink, url);
+    // case-1: when link is this growi's page url, return pathname only
+    let reshapedLink = url != null && url.origin === window.location.origin
+      ? decodeURIComponent(url.pathname)
+      : link;
 
     // case-3
-    if (!isUrl && !reshapedLink.startsWith('/') && reshapedLink !== '') {
+    if (!isFqcn && !reshapedLink.startsWith('/') && reshapedLink !== '') {
       isUseRelativePath = true;
       const rootPath = this.getRootPath(type);
       reshapedLink = path.resolve(rootPath, reshapedLink);
@@ -118,12 +129,6 @@ class LinkEditModal extends React.PureComponent {
     });
   }
 
-  // return path name of link if link is this growi page url, else return original link.
-  convertUrlToPathIfPageUrl(link, url) {
-    // when link is this growi's page url, url.origin === window.location.origin and return path name
-    return url.origin === window.location.origin ? decodeURI(url.pathname) : link;
-  }
-
   cancel() {
     this.hide();
   }
@@ -161,11 +166,11 @@ class LinkEditModal extends React.PureComponent {
     let previewError = '';
 
     if (path.startsWith('/')) {
-      const pathWithoutFragment = new URL(path, 'http://dummy').pathname;
-      const isPermanentLink = validator.isMongoId(pathWithoutFragment.slice(1));
-      const pageId = isPermanentLink ? pathWithoutFragment.slice(1) : null;
-
       try {
+        const pathWithoutFragment = new URL(path, 'http://dummy').pathname;
+        const isPermanentLink = validator.isMongoId(pathWithoutFragment.slice(1));
+        const pageId = isPermanentLink ? pathWithoutFragment.slice(1) : null;
+
         const { data } = await apiv3Get('/page', { path: pathWithoutFragment, page_id: pageId });
         const { page } = data;
         markdown = page.revision.body;

+ 11 - 2
packages/app/src/components/ReactMarkdownComponents/Header.tsx

@@ -6,12 +6,15 @@ import { useRouter } from 'next/router';
 import { Element } from 'react-markdown/lib/rehype-filter';
 
 import { useIsGuestUser, useIsSharedUser, useShareLinkId } from '~/stores/context';
+import loggerFactory from '~/utils/logger';
 
 import { NextLink } from './NextLink';
 
 import styles from './Header.module.scss';
 
 
+const logger = loggerFactory('growi:components:Header');
+
 declare global {
   // eslint-disable-next-line vars-on-top, no-var
   var globalEmitter: EventEmitter;
@@ -67,8 +70,14 @@ export const Header = (props: HeaderProps): JSX.Element => {
   const CustomTag = `h${level}` as keyof JSX.IntrinsicElements;
 
   const activateByHash = useCallback((url: string) => {
-    const hash = (new URL(url, 'https://example.com')).hash.slice(1);
-    setActive(hash === id);
+    try {
+      const hash = (new URL(url, 'https://example.com')).hash.slice(1);
+      setActive(hash === id);
+    }
+    catch (err) {
+      logger.debug(err);
+      setActive(false);
+    }
   }, [id]);
 
   // init

+ 13 - 4
packages/app/src/components/ReactMarkdownComponents/NextLink.tsx

@@ -3,16 +3,25 @@ import { Link as ScrollLink } from 'react-scroll';
 
 import { DEFAULT_AUTO_SCROLL_OPTS } from '~/client/util/smooth-scroll';
 import { useSiteUrl } from '~/stores/context';
+import loggerFactory from '~/utils/logger';
+
+
+const logger = loggerFactory('growi:components:NextLink');
 
 const isAnchorLink = (href: string): boolean => {
   return href.toString().length > 0 && href[0] === '#';
 };
 
 const isExternalLink = (href: string, siteUrl: string | undefined): boolean => {
-  const baseUrl = new URL(siteUrl ?? 'https://example.com');
-  const hrefUrl = new URL(href, baseUrl);
-
-  return baseUrl.host !== hrefUrl.host;
+  try {
+    const baseUrl = new URL(siteUrl ?? 'https://example.com');
+    const hrefUrl = new URL(href, baseUrl);
+    return baseUrl.host !== hrefUrl.host;
+  }
+  catch (err) {
+    logger.debug(err);
+    return false;
+  }
 };
 
 type Props = Omit<LinkProps, 'href'> & {

+ 20 - 8
packages/app/src/server/middlewares/safe-redirect.js → packages/app/src/server/middlewares/safe-redirect.ts

@@ -4,31 +4,43 @@
  * Usage: app.use(require('middlewares/safe-redirect')(['example.com', 'some.example.com:8080']))
  */
 
+import {
+  Request, Response, NextFunction,
+} from 'express';
+
 import loggerFactory from '~/utils/logger';
 
 const logger = loggerFactory('growi:middleware:safe-redirect');
 
 /**
  * Check whether the redirect url host is in specified whitelist
- * @param {Array<string>} whitelistOfHosts
- * @param {string} redirectToFqdn
  */
-function isInWhitelist(whitelistOfHosts, redirectToFqdn) {
+function isInWhitelist(whitelistOfHosts: string[], redirectToFqdn: string): boolean {
   if (whitelistOfHosts == null || whitelistOfHosts.length === 0) {
     return false;
   }
 
-  const redirectUrl = new URL(redirectToFqdn);
-  return whitelistOfHosts.includes(redirectUrl.hostname) || whitelistOfHosts.includes(redirectUrl.host);
+  try {
+    const redirectUrl = new URL(redirectToFqdn);
+    return whitelistOfHosts.includes(redirectUrl.hostname) || whitelistOfHosts.includes(redirectUrl.host);
+  }
+  catch (err) {
+    logger.warn(err);
+    return false;
+  }
 }
 
 
-module.exports = (whitelistOfHosts) => {
+type ResWithSafeRedirect = Response & {
+  safeRedirect: (redirectTo?: string) => void,
+}
+
+module.exports = (whitelistOfHosts: string[]) => {
 
-  return function(req, res, next) {
+  return function(req: Request, res: ResWithSafeRedirect, next: NextFunction) {
 
     // extend res object
-    res.safeRedirect = function(redirectTo) {
+    res.safeRedirect = function(redirectTo?: string) {
       if (redirectTo == null) {
         return res.redirect('/');
       }

+ 10 - 3
packages/remark-lsx/src/services/renderer/lsx.ts

@@ -4,13 +4,14 @@ import { pathUtils } from '@growi/core';
 import { remarkGrowiDirectivePluginType } from '@growi/remark-growi-directive';
 import { Schema as SanitizeOption } from 'hast-util-sanitize';
 import { selectAll, HastNode } from 'hast-util-select';
+import isAbsolute from 'is-absolute-url';
 import { Plugin } from 'unified';
 import { visit } from 'unist-util-visit';
 
 const NODE_NAME_PATTERN = new RegExp(/ls|lsx/);
 const SUPPORTED_ATTRIBUTES = ['prefix', 'num', 'depth', 'sort', 'reverse', 'filter', 'except'];
 
-const { hasHeadingSlash } = pathUtils;
+const { addHeadingSlash, hasHeadingSlash } = pathUtils;
 
 type DirectiveAttributes = Record<string, string>
 
@@ -66,10 +67,16 @@ export type LsxRehypePluginParams = {
   pagePath?: string,
 }
 
-const pathResolver = (relativeHref: string, basePath: string): string => {
+const pathResolver = (href: string, basePath: string): string => {
+  // exclude absolute URL
+  if (isAbsolute(href)) {
+    // remove scheme
+    return href.replace(/^(.+?):\/\//, '/');
+  }
+
   // generate relative pathname
   const baseUrl = new URL(pathUtils.addTrailingSlash(basePath), 'https://example.com');
-  const relativeUrl = new URL(relativeHref, baseUrl);
+  const relativeUrl = new URL(href, baseUrl);
 
   return relativeUrl.pathname;
 };