CVE-2019-8903: Total.js requestcontinue Dir Traversal.md

CVE-2019-8903: Total.js requestcontinue Directory Traversal Vulnerability

Background

Total.js is a free web application framework for building websites and web applications using JavaScript, HTML, and CSS. It is based on the Node.js platform.

A directory traversal vulnearbility was found by Riccardo Krauter, and reported to Total.js on February 2019. A Metasploit module was submitted by Fabio Cogno on March. The vulnerability can be found in the framework's F.$requestcontinue function in index.js, which can be exploited in order to download files from the server without authentication.

Vulnerability Analysis

Environment Setup

For debugging purposes, the CMS application that comes with Total.js works best to investigate the issue. In order to recreate the vulnerable setup, first install Total.js to the vulnerable version:

$ npm install total.js@3.2.2

Next, install the CMS:

$ git clone https://github.com/totaljs/cms.git
$ cd cms && npm install

And finally, start the application:

$ node debug.js

The application should start in debug mode, and tell you important information such as the version of Total.js, Node.js, and the CMS version. Always make sure these are verified before proceeding.

Proof-of-Concept

Now that we have a vulnerble setup, we can look into the vulnerability. Since there is already a Metasploit module, we can use that to learn what the HTTP request should look like to trigger the bug:

traverse = '%2e%2e%2f' * datastore['DEPTH']
uri = normalize_uri(target_uri.path) + traverse + datastore['FILE']

res = send_request_cgi('method' => 'GET', 'uri' => uri)

It seems in order to trigger the bug, all we have to do is sending ../ in the GET request. The next question we want to ask is, how does the web application process the request?

Debugging Node.js

Like debugging any other applications, we begin by finding a good starting point to debug. In this case, after learning how Total.js starts a web server from the source, this line is where we begin (in index.js):

F.server = http.createServer(F.listener);

F.listener is actually a function that is meant for request processing according to its code comment. This comment is a great direction for us so we want to start off here.

One of the proper ways to debug a Node.js application is by using the Chrome browser as a debugger. This can be done by starting off the web server this way:

$ node inspect release.js

Next, open Chrome, and then enter the following in the address bar:

about:inspect

From there, it will be obvious that there is a Node.js target waiting to be debugged. At this point, we can add a bunch of breakpoints and observe how our input is processed.

By adding some breakpoints starting from F.listener, we eventually map out the execution flow:

F.listener -> F.$requestcontinue -> PROTO.$total_file -> PROTO.$total_endfile -> PROTO.continue

The PROTO.continue function is where the filename is retrieved from the GET request, specifically this line (index.js:15924):

filename = F.onMapping(name, name, true, true);

Next, the function retrieves the file by performing the following (index.js:15944):

res.options.filename = filename;
res.$file();
return res;

And res.$file() basically uses createReadStream to read the file, which is part of the Node.js filesystem API. The return statement at the end of the $PROTO.continue function is what the attacker gets in the end.

Ineffective Past Mitigation

It is also worth mentioning that during the vulnerability analysis, I spotted a past attempt of the author trying to prevent directory traveral attacks, but clearly failed. We can see this mitigitation code in the F.$requestcontinue function, where is part of the execution flow the attacker is on:

F.$requestcontinue = function(req, res, headers) {

	if (!req || !res || res.headersSent || res.success)
		return;

	// Validates if this request is the file (static file)
	if (req.isStaticFile) {

		// Stops path travelsation outside of "public" directory
		// A potential security issue
		if (req.uri.pathname.indexOf('./') !== -1) {
			req.$total_status(404);
			return;
		}

The reason this mitigation doesn't work is because it is only effective at preventing a traversal attack like this:

../../

But it forgets to account for the fact HTTP also supports character escaping, so that means this would still work too:

/%2e%2e%2fpackage.json

Patch Information

It seems after the vulnerability was reported, the author attempted to fix the problem twice. The first one was:

	// Removes directory browsing
	for (var i = 0; i < req.url.length; i++) {
		if (req.url[i] === '.' && req.url[i + 1] === '/')
			beg = i + 1;
		else if (req.url[i] === '?')
			break;
	}

	if (beg)
		req.url = req.url.substring(beg);

Well, that wouldn't work, because it is still checking . and /. But the following day, the author rewrote the patch:

		for (var i = 0; i < req.uri.pathname.length; i++) {
			var c = req.uri.pathname[i];
			var n = req.uri.pathname[i + 1];
			if ((c === '.' && n === '/') || (c === '%' && n === '2' && req.uri.pathname[i + 2] === 'e')) {
				req.$total_status(404);
				return;
			}
		}

Although there are multiple ways to fix it, as long as % is being checked, it is much better.

Another possible and cleaner way for the fix is actually unescaping the value for req.uri.pathname before doing a indexOf for the old patch, which would be an one-line fix. Regression not investigated though.