integer overflow in indentWidth allocation in binding.cpp

[anticomputer]

• NPM version (npm -v):
  6.14.6

• Node version (node -v):
  10.22.0

• Node Process (node -p process.versions):
  { http_parser: '2.9.3',
  node: '10.22.0',
  v8: '6.8.275.32-node.58',
  uv: '1.34.2',
  zlib: '1.2.11',
  brotli: '1.0.7',
  ares: '1.15.0',
  modules: '64',
  nghttp2: '1.41.0',
  napi: '6',
  openssl: '1.1.1g',
  icu: '64.2',
  unicode: '12.1',
  cldr: '35.1',
  tz: '2019c' }

• Node Platform (node -p process.platform):
  linux

• Node architecture (node -p process.arch):
  x64

• node-sass version (node -p "require('node-sass').info"):
  node-sass 4.14.1 (Wrapper) [JavaScript]
  libsass 3.5.5 (Sass Compiler) [C/C++]

• npm node-sass versions (npm ls node-sass):

/home/anticomputer
└── [email protected]

Summary:

There exists an integer overflow in binding.cpp in handling the indentWidth option. This integer overflow leads to an underallocation of heap memory. This bug does not represent a vulnerability as this input is likely not supplied in a remote input scenario and a std::string exception will occur before any heap corruption happens. Even if heap corruption were to occur, it would be a very limited control overwrite with either tab or space characters based on a very large indent_len with a low likelihood of exploitability.

Details:

In node-sass/src/binding.cpp we observe the following:

 int indent_len = Nan::To<int32_t>(
    Nan::Get(
        options,
        Nan::New("indentWidth").ToLocalChecked()
    ).ToLocalChecked()).FromJust();

[1]
  ctx_w->indent = (char*)malloc(indent_len + 1);

  strcpy(ctx_w->indent, std::string(
[2]
    indent_len,
    Nan::To<int32_t>(
        Nan::Get(
            options,
            Nan::New("indentType").ToLocalChecked()
        ).ToLocalChecked()).FromJust() == 1 ? '\t' : ' '

At [1] we see a user input controlled 32bit integer value is used to allocate memory. If this user supplied integer can be -1, the integer arithmetic expression indent_len + 1 would result in 0. At [2] the original negative value is then used to create a tab or space string of indent_len characters.

At the JS API level we note indentWidth is retrieved as follows:

/**
 * Get indent width
 *
 * @param {Object} options
 * @api private
 */

function getIndentWidth(options) {
  var width = parseInt(options.indentWidth) || 2;

  return width > 10 ? 2 : width;
}

The intent here is to ensure indentWidth is >= 2 or <= 10, however only the upper bound is checked and parseInt allows us to supply a negative value, e.g.:

var sass = require('node-sass')
var result = sass.renderSync({
	data: `h1 { font-size: 40px; }`,
	indentWidth: -1
});

This will trigger the integer overwrap as described, and then throw an exception in std::string.

anticomputer@dc1:~$ node sass.js 
terminate called after throwing an instance of 'std::length_error'
  what():  basic_string::_S_create
Aborted (core dumped)
anticomputer@dc1:~$ 

Remediation:

Ensure that both the lower and upper bounds of the user supplied indentWidth value are checked before handing this value off to the lower level binding.