search

vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.57k stars 26.82k forks source link

Production build not using custom server #5289

Closed richbai90 closed 6 years ago

richbai90 commented 6 years ago

TLDR;

A custom server using express with a nonce is working when running in dev but gives a type error after building successfully, when running in production.

Full Description

I have a custom server that works great in development. The main changes are that I'm using an express server with lusca for CSP and some custom middleware to prevent access to pages outside of the web directory. Per the lusca documentation, the nonce is generated on every request and placed in res.locals Where I get it in a custom _document page.

EDIT: simplified server.js based on comments

Server.js

/**
 * Use a custom server with next to allow for custom routing using next-routes
 * And additional security using lusca.
 */

const next = require('next');
const express = require('express');
const session = require('express-session');
const lusca = require('lusca');

const routes = require('./src/routes');

const app = next({ dev: process.env.NODE_ENV !== 'production' });
const handler = routes.getRequestHandler(app);
const env = process.env.NODE_ENV || 'dev';

// bypassing all the custom server configuration for the purposes of testing

const configureApp = (server) => {
  server.use(session({
    secret: 'Kuxo9R4E1W+fzC9a/aJohGnCCJcRlnXA1VXhUNxiYzLWtDt1xamoQh/2E68gFUycrNa674Q3gHhbaKilVz07VSA/DZcjJ6LoEUTpuWHNAbKgILA26o2YyuN1PafG/ZzsNdPCfmf9IRrUEBupUHGpZcOje5p6yy0GjLkVBg71XEQ=',
    resave: true,
    saveUninitialized: true,
  }));

  // security settings
  server.use(lusca({
    csrf: true,
    csp: {
      styleNonce: true,
      scriptNonce: true,
      policy: {
        'default-src': env === 'dev' ? "'unsafe-eval' 'self'" : "'self'",
      },
    },
    xframe: 'SAMEORIGIN',
    hsts: { maxAge: 31536000, includeSubDomains: true, preload: true },
    xssProtection: true,
    nosniff: true,
    referrerPolicy: 'same-origin',
  }));

  server.use(handler);

  return server;
};

app.prepare().then(() => {
  const server = express();
  configureApp(server).listen(3000);
  // server.use(handler);
  // server.listen(3000);
});

_document.tsx

import { Request, Response } from 'express';
import { ApplicationDocumentContext } from 'next';
import Document, { AnyPageProps, Head, Main, NextDocumentContext, NextScript, WebclientDocumentProps } from 'next/document';
import Error from 'next/error';
import React, { ReactElement, ReactNode } from 'react';
import getPageContext from 'src/mui/getPageContext';
import flush from 'styled-jsx/server';

declare module 'next' {
  interface ExpressContext {
    req?: Request;
    res?: Response;
  }

  export type ApplicationDocumentContext = ApplicationContext & NextDocumentContext

  export type ApplicationContext = Omit<NextContext, 'req'|'res'> & ExpressContext

}

declare module 'next/document' {
  interface MuiDocumentProps {
    styles?: Array<ReactElement<any>>|JSX.Element
  }

  export type WebclientDocumentProps = Omit<DocumentProps, 'styles'> & MuiDocumentProps
}

class MyDocument extends Document {

  public static getInitialProps(ctx : ApplicationDocumentContext ) : WebclientDocumentProps {
    // Resolution order
    //
    // On the server:
    // 1. app.getInitialProps
    // 2. page.getInitialProps
    // 3. document.getInitialProps
    // 4. app.render
    // 5. page.render
    // 6. document.render
    //
    // On the server with error:
    // 1. document.getInitialProps
    // 2. app.render
    // 3. page.render
    // 4. document.render
    //
    // On the client
    // 1. app.getInitialProps
    // 2. page.getInitialProps
    // 3. app.render
    // 4. page.render

    // Render app and page and get the context of the page with collected side effects.
    let pageContext = getPageContext(); // TODO 09/25/2018 Rich Baird : Fix this workaround, pageContext should always come from _app.js props
    const page = ctx.renderPage(Component => {
      const WrappedComponent = (props : (AnyPageProps & {children? : ReactNode}) ) => {
        pageContext = props.pageContext;
        return <Component {...props} />;
      };
      return WrappedComponent;
    });

    let nonce = '';
    let statusCode = 200;
    if(ctx.res) {
      nonce = ctx.res.locals.nonce!;
      statusCode = ctx.res.statusCode;
    }

    return {
      nonce,
      ...page,
      pageContext,
      statusCode,
      // Styles fragment is rendered after the app and page rendering finish.
      styles: (
        <>
          <style
            nonce={nonce}
            id="jss-server-side"
            // eslint-disable-next-line react/no-danger
            dangerouslySetInnerHTML={{ __html: (pageContext!).sheetsRegistry.toString() }}
          />
          {flush() || null}
        </>
      ),
    };
  };

  public render() {
    const { pageContext, nonce, statusCode } = this.props;
    if (statusCode > 200) { return <Error statusCode={this.props.statusCode} />; }
    return (
      <html lang="en" dir="ltr">
        <Head>
          <title>My page</title>
          <meta charSet="utf-8" />
          {/* Use minimum-scale=1 to enable GPU rasterization */}
          <meta
            name="viewport"
            content={
              'user-scalable=0, initial-scale=1, '
              + 'minimum-scale=1, width=device-width, height=device-height'
            }
          />
          {/* PWA primary color */}
          <meta name="theme-color" content={pageContext.theme.palette.primary.main} />
          <meta property="csp-nonce" content={nonce} />
          <link
            rel="stylesheet"
            href="https://fonts.googleapis.com/css?family=Roboto:300,400,500"
          />
          <link
            href="https://fonts.googleapis.com/icon?family=Material+Icons"
            rel="stylesheet"
          />
        </Head>
        <body>
          <Main />
          <NextScript nonce={nonce} />
        </body>
      </html>
    );
  }
}

export default MyDocument;

There are a lot of moving parts and variables with this setup, I'm using typescript and material-ui among other things but it all works perfectly when I run it using npm run dev but if I do npm run build && npm start It builds fine but I get an error

TypeError: Cannot read property 'nonce' of undefined at Function.getInitialProps (/home/node/webclient-appsvr/.next/server/static/CAR87GSCQfuY4qyqbJ2sz/pages/_document.js:402:32) at _callee$ (/home/node/webclient-appsvr/node_modules/next/dist/lib/utils.js:135:30) at tryCatch (/home/node/webclient-appsvr/node_modules/regenerator-runtime/runtime.js:62:40) at Generator.invoke [as _invoke] (/home/node/webclient-appsvr/node_modules/regenerator-runtime/runtime.js:288:22) at Generator.prototype.(anonymous function) [as next] (/home/node/webclient-appsvr/node_modules/regenerator-runtime/runtime.js:114:21) at asyncGeneratorStep (/home/node/webclient-appsvr/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:5:24) at _next (/home/node/webclient-appsvr/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:27:9) at /home/node/webclient-appsvr/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:34:7 at new Promise () at new F (/home/node/webclient-appsvr/node_modules/core-js/library/modules/_export.js:36:28)

kyle-mccarthy commented 6 years ago

Whats the motive behind maintaining a whitelist like that? Did you disable the file system routing or are you still using that?

richbai90 commented 6 years ago

If there is a configuration that I'm missing I don't know. I ran my app through a vulnerability scan and it told me that it could access files outside of the webroot. So I came up with this solution.

kyle-mccarthy commented 6 years ago

Can you provide some more information on the scan that you used and what files it said that it could access? You shouldn't need to whitelist everything like that. Assuming that you are using a custom server, and are not using the filesystem routing, my approach is to:

  1. Allow my server routes to try and match the incoming page. If there is a match use app.render(req, res, '/my/view', query) to render the page.
  2. If the a matching route didn't exist on the server, let the request fall through to default next request handler. Which is returned by app.getRequestHandler() and then can be called like this handler(req, res, query). If it isn't a next build asset, the request will 404.

edit: Also how are you running this in production? What is your start command? If it is yarn start or npm start what does that map to in your package.json?

richbai90 commented 6 years ago

I'm using OWASP ZAP scanner. Without any custom server implementation beyond an out of the box un-configured express server, here is what I get while running in development. To fix the xss and xframe concerns I implemented a CSP with a nonce to allow next-script to execute. For the path traversal I implemented the whitelist strategy. In production on my local machine I run npm start which simply maps to next start when I run it in docker I run node node_modules/next/dist/bin/next start. It's a best practice in docker to avoid using npm start so that you may reduce the number of running processes and make node, not npm the entry point.

ZAP Scanning Report

Summary of Alerts

Risk Level Number of Alerts
High 1
Medium 2
Low 2
Informational 0

Alert Detail

Path Traversal

High (Medium)

Description

The Path Traversal attack technique allows an attacker access to files, directories, and commands that potentially reside outside the web document root directory. An attacker may manipulate a URL in such a way that the web site will execute or reveal the contents of arbitrary files anywhere on the web server. Any device that exposes an HTTP-based interface is potentially vulnerable to Path Traversal.

Most web sites restrict user access to a specific portion of the file-system, typically called the "web document root" or "CGI root" directory. These directories contain the files intended for user access and the executable necessary to drive web application functionality. To access files or execute commands anywhere on the file-system, Path Traversal attacks will utilize the ability of special-characters sequences.

The most basic Path Traversal attack uses the "../" special-character sequence to alter the resource location requested in the URL. Although most popular web servers will prevent this technique from escaping the web document root, alternate encodings of the "../" sequence may help bypass the security filters. These method variations include valid and invalid Unicode-encoding ("..%u2216" or "..%c0%af") of the forward slash character, backslash characters ("..\") on Windows-based servers, URL encoded characters "%2e%2e%2f"), and double URL encoding ("..%255c") of the backslash character.

Even if the web server properly restricts Path Traversal attempts in the URL path, a web application itself may still be vulnerable due to improper handling of user-supplied input. This is a common problem of web applications that use template mechanisms or load static text from files. In variations of the attack, the original URL parameter value is substituted with the file name of one of the web application's dynamic scripts. Consequently, the results can reveal source code because the file is interpreted as text instead of an executable script. These techniques often employ additional special characters such as the dot (".") to reveal the listing of the current working directory, or "%00" NULL characters in order to bypass rudimentary file extension checks.

Instances: 2

Solution

Assume all input is malicious. Use an "accept known good" input validation strategy, i.e., use a whitelist of acceptable inputs that strictly conform to specifications. Reject any input that does not strictly conform to specifications, or transform it into something that does. Do not rely exclusively on looking for malicious or malformed inputs (i.e., do not rely on a blacklist). However, blacklists can be useful for detecting potential attacks or determining which inputs are so malformed that they should be rejected outright.

When performing input validation, consider all potentially relevant properties, including length, type of input, the full range of acceptable values, missing or extra inputs, syntax, consistency across related fields, and conformance to business rules. As an example of business rule logic, "boat" may be syntactically valid because it only contains alphanumeric characters, but it is not valid if you are expecting colors such as "red" or "blue."

For filenames, use stringent whitelists that limit the character set to be used. If feasible, only allow a single "." character in the filename to avoid weaknesses, and exclude directory separators such as "/". Use a whitelist of allowable file extensions.

Warning: if you attempt to cleanse your data, then do so that the end result is not in the form that can be dangerous. A sanitizing mechanism can remove characters such as '.' and ';' which may be required for some exploits. An attacker can try to fool the sanitizing mechanism into "cleaning" data into a dangerous form. Suppose the attacker injects a '.' inside a filename (e.g. "sensi.tiveFile") and the sanitizing mechanism removes the character resulting in the valid filename, "sensitiveFile". If the input data are now assumed to be safe, then the file may be compromised.

Inputs should be decoded and canonicalized to the application's current internal representation before being validated. Make sure that your application does not decode the same input twice. Such errors could be used to bypass whitelist schemes by introducing dangerous inputs after they have been checked.

Use a built-in path canonicalization function (such as realpath() in C) that produces the canonical version of the pathname, which effectively removes ".." sequences and symbolic links.

Run your code using the lowest privileges that are required to accomplish the necessary tasks. If possible, create isolated accounts with limited privileges that are only used for a single task. That way, a successful attack will not immediately give the attacker access to the rest of the software or its environment. For example, database applications rarely need to run as the database administrator, especially in day-to-day operations.

When the set of acceptable objects, such as filenames or URLs, is limited or known, create a mapping from a set of fixed input values (such as numeric IDs) to the actual filenames or URLs, and reject all other inputs.

Run your code in a "jail" or similar sandbox environment that enforces strict boundaries between the process and the operating system. This may effectively restrict which files can be accessed in a particular directory or which commands can be executed by your software.

OS-level examples include the Unix chroot jail, AppArmor, and SELinux. In general, managed code may provide some protection. For example, java.io.FilePermission in the Java SecurityManager allows you to specify restrictions on file operations.

This may not be a feasible solution, and it only limits the impact to the operating system; the rest of your application may still be subject to compromise.

Reference

CWE Id : 22

WASC Id : 33

Source ID : 1

X-Frame-Options Header Not Set

Medium (Medium)

Description

X-Frame-Options header is not included in the HTTP response to protect against 'ClickJacking' attacks.

Instances: 1

Solution

Most modern Web browsers support the X-Frame-Options HTTP header. Ensure it's set on all web pages returned by your site (if you expect the page to be framed only by pages on your server (e.g. it's part of a FRAMESET) then you'll want to use SAMEORIGIN, otherwise if you never expect the page to be framed, you should use DENY. ALLOW-FROM allows specific websites to frame the web page in supported web browsers).

Reference

CWE Id : 16

WASC Id : 15

Source ID : 3

Application Error Disclosure

Medium (Medium)

Description

This page contains an error/warning message that may disclose sensitive information like the location of the file that produced the unhandled exception. This information can be used to launch further attacks against the web application. The alert could be a false positive if the error message is found inside a documentation page.

Instances: 1

Solution

Review the source code of this page. Implement custom error pages. Consider implementing a mechanism to provide a unique error reference/identifier to the client (browser) while logging the details on the server side and not exposing them to the user.

Reference

CWE Id : 200

WASC Id : 13

Source ID : 3

Web Browser XSS Protection Not Enabled

Low (Medium)

Description

Web Browser XSS Protection is not enabled, or is disabled by the configuration of the 'X-XSS-Protection' HTTP response header on the web server

Instances: 4

Solution

Ensure that the web browser's XSS filter is enabled, by setting the X-XSS-Protection HTTP response header to '1'.

Other information

The X-XSS-Protection HTTP response header allows the web server to enable or disable the web browser's XSS protection mechanism. The following values would attempt to enable it:

X-XSS-Protection: 1; mode=block

X-XSS-Protection: 1; report=http://www.example.com/xss

The following values would disable it:

X-XSS-Protection: 0

The X-XSS-Protection HTTP response header is currently supported on Internet Explorer, Chrome and Safari (WebKit).

Note that this alert is only raised if the response body could potentially contain an XSS payload (with a text-based content type, with a non-zero length).

Reference

CWE Id : 933

WASC Id : 14

Source ID : 3

X-Content-Type-Options Header Missing

Low (Medium)

Description

The Anti-MIME-Sniffing header X-Content-Type-Options was not set to 'nosniff'. This allows older versions of Internet Explorer and Chrome to perform MIME-sniffing on the response body, potentially causing the response body to be interpreted and displayed as a content type other than the declared content type. Current (early 2014) and legacy versions of Firefox will use the declared content type (if one is set), rather than performing MIME-sniffing.

Instances: 7

Solution

Ensure that the application/web server sets the Content-Type header appropriately, and that it sets the X-Content-Type-Options header to 'nosniff' for all web pages.

If possible, ensure that the end user uses a standards-compliant and modern web browser that does not perform MIME-sniffing at all, or that can be directed by the web application/web server to not perform MIME-sniffing.

Other information

This issue still applies to error type pages (401, 403, 500, etc) as those pages are often still affected by injection issues, in which case there is still concern for browsers sniffing pages away from their actual content type.

At "High" threshold this scanner will not alert on client or server error responses.

Reference

CWE Id : 16

WASC Id : 15

Source ID : 3

kyle-mccarthy commented 6 years ago

You shouldn't be running the scanner in development mode. Development mode exposes access points and additional URLs that aren't available in a production ENV. If you remove your whitelist logic and allow for next to handle resolving the requests and mapping them to files, it should at least remove the path traversal alert and your routes should work.

richbai90 commented 6 years ago

Good to know, I'll give that a shot and let you know if it helps with the nonce issue

richbai90 commented 6 years ago

Just had time to update the app, see the new server.js file in the issue. Unfortunately it did nothing to help with the error.

TypeError: Cannot read property 'nonce' of undefined at Function.getInitialProps (/Users/rich/code/webclient-appsvr/.next/server/static/TtAPU~ueU_78OvpI7QS5T/pages/_document.js:402:32) at _callee$ (/Users/rich/code/webclient-appsvr/node_modules/next/dist/lib/utils.js:135:30) at tryCatch (/Users/rich/code/webclient-appsvr/node_modules/regenerator-runtime/runtime.js:62:40) at Generator.invoke [as _invoke] (/Users/rich/code/webclient-appsvr/node_modules/regenerator-runtime/runtime.js:288:22) at Generator.prototype.(anonymous function) [as next] (/Users/rich/code/webclient-appsvr/node_modules/regenerator-runtime/runtime.js:114:21) at asyncGeneratorStep (/Users/rich/code/webclient-appsvr/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:5:24) at _next (/Users/rich/code/webclient-appsvr/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:27:9) at /Users/rich/code/webclient-appsvr/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:34:7 at new Promise () at new F (/Users/rich/code/webclient-appsvr/node_modules/core-js/library/modules/_export.js:36:28)

What it's complaining about as near as I can tell is here:

nonce = ctx.res.locals.nonce!;

and it's insisting that ctx.res.locals is undefined even though it works great in dev.

kyle-mccarthy commented 6 years ago

@richbai90 See this gist https://gist.github.com/kyle-mccarthy/d04f94f4cc972a4c9b43a86d74ce9d02