SQL injection attempts killls Etherpad lite

[fpoulain]

Hi,

On our server we were getting some Etherpad outage. We relied it to a nasty query:

https://pad.bling.org/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&vLtF%3D6904%20AND%201%3D1%20UNION%20ALL%20SELECT%201%2CNULL%2C%27%3Cscript%3Ealert%28%22XSS%22%29%3C%2Fscript%3E%27%2Ctable_name%20FROM%20information_schema.tables%20WHERE%202%3E1--%2F%2A%2A%2F%3B%20EXEC%20xp_cmdshell%28%27cat%20..%2F..%2F..%2Fetc%2Fpasswd%27%29%23

A "minimal" query example:

https://pad.bling.org/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&vLtF%3D6904%20AND%201%3D1%20UNION%20ALL%20SELECT%201%2CNULL%2C%27%3Cscript%3Ealert(%22XSS%22)%3C%2Fscript%3E%27

This provoke an immediate crash:

oct. 23 18:17:19 pad.bling.org run.sh[8976]: [2018-10-23 18:17:19.994] [ERROR] console - Error: ENAMETOOLONG: name too long, open '/var/www/etherpad-lite/var/minified_L2phdmFzY3JpcHRzL2xpYi9lcF9ldGhlcn
oct. 23 18:17:19 pad.bling.org run.sh[8976]:     at Error (native)
oct. 23 18:17:19 pad.bling.org run.sh[8976]: [2018-10-23 18:17:19.995] [INFO] console - graceful shutdown...
oct. 23 18:17:20 pad.bling.org run.sh[8976]: [2018-10-23 18:17:20.091] [INFO] console - db sucessfully closed.

We are running the 1.7.0 flavor on Debian Stretch with node v6.14.4 and no specific customization. We reproduced the behavior on two independents Etherpad installation.

[muxator]

That's true, indeed. Thanks for the precious info, @fpoulain, much appreciated!

[JohnMcLear]

Hey guys, just a reminder about responsible disclosure. Posting publicly without giving us chance to pick can be quite dangerous.

[fpoulain]

Hey guys, just a reminder about responsible disclosure. Posting publicly without giving us chance to pick can be quite dangerous.

Hum ... actually it has been already disclosed because it is used by some nasty guys.

Also, I don't see how to report it in a non public way. The project description don't mention anyway private feedback loop for security issues. How do you think would I had reported it?

[JohnMcLear]

Afaik we have a responsible disclosure policy. I thought it was on etherpad.org and the github readme.

---

From: François Poulain notifications@github.com Sent: Monday, 21 January 2019 11:16 am To: ether/etherpad-lite Cc: John McLear; Comment Subject: Re: [ether/etherpad-lite] SQL injection attempts killls Etherpad lite (#3502)

Hey guys, just a reminder about responsible disclosure. Posting publicly without giving us chance to pick can be quite dangerous.

Hum ... actually it has been already disclosed because it is used by some nasty guys.

Also, I don't see how to report it in a non public way. The project description don't mention anyway private feedback loop for security issues. How do you think would I had reported it?

- You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/ether/etherpad-lite/issues/3502#issuecomment-456039980, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AANewII6dbcPV2hlcP4uA5c4OHESwQJuks5vFaK4gaJpZM4X1_tz.

[fpoulain]

It could be a good idea to add some invitation "found a security issue? tell us about it via ...". Before opening this issue I spent few minutes on github's readme and on etherpad.org without finding such an invitation. Also, as a non native english reader, I could have missed the good terms to seek for.

[JohnMcLear]

Noted. Tnx

---

From: François Poulain notifications@github.com Sent: Monday, 21 January 2019 2:35 pm To: ether/etherpad-lite Cc: John McLear; Comment Subject: Re: [ether/etherpad-lite] SQL injection attempts killls Etherpad lite (#3502)

It could be a good idea to add some invitation "found a security issue? tell us about it via ...". Before opening this issue I spent few minutes on github and on etherpad.org without finding such an invitation. Also, as a non native english reader, I could have missed the good terms to seek for.

- You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/ether/etherpad-lite/issues/3502#issuecomment-456096384, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AANewFZlv6xtuszRP2CCBwWoDHGgTrFwks5vFdGEgaJpZM4X1_tz.

[marksteward]

I couldn't find one either, but I did find you requesting a PR to add one https://github.com/ether/etherpad-lite/issues/2499.

Perhaps consider just creating a https://securitytxt.org/ as a quick fix?

[JohnMcLear]

Offending code is somewhere in here: https://github.com/ether/yajsml/blob/master/server.js#L98

[JohnMcLear]

Disclaimer: I'm not clever enough to resolve this. I need someone with expertise to help!

[tomnomnom]

Hey! Doing some looking into this now.

First off: to fend off any fears about SQLi: this has nothing to do with SQL injection, as evidenced by this alternative payload that also causes the same crash:

/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&footlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootlefootle

This is a filename length issue in the caching middleware.

Currently the cache key is generated as follows:

var cacheKey = Buffer.from(path).toString('base64').replace(/[/+=]/g, '');

This causes the cache key length to be controlled by the length of the path.

This is a problem when the cache key is used as a filename on disk, as many filesystems limit filename length to 255 characters. This is why there's an ENAMETOOLONG error in the log.

To remedy this, I suggest making the cache key a hash of the path instead of the base64 encoded version. This will ensure a fixed-length filename.

I'll submit a PR that does this shortly.

[muxator]

Hi, @tomnomnom, this is clever. It completely makes sense that cache keys are of fixed length. In this way they stay dependent on the content, with practically non existent risk of collisions.

I'll have a look tonight. Well done!

[muxator]

Merged the PR, many thanks!

A further note about this: from the nodejs docs we may have to consider the possibility of the crypto module being unavailable:

It is possible for Node.js to be built without including support for the crypto module. In such cases, calling require('crypto') will result in an error being thrown

Maybe embedded platforms may not have it (e.g.: ARM, MIPS, Raspberry PIs & similar)? I do not know. A description of such scenarios can be found here:

• running non-standard node in a resource- or security-constrained environment (see node#5611 for their explicit support of this scenario)
• running in emulated environment (browserify, webpack etc.)
• building node from source and omitting openssl/crypto for random reason (see StackOverflow question or another nodejs post)

Anyway, the TypeScript guys dealt with this same issue in https://github.com/microsoft/TypeScript/issues/19100, and they resolved it in an elegant way in https://github.com/microsoft/TypeScript/commit/9677b0641cc5ba7d8b701b4f892ed7e54ceaee9a.

If the importing crypto fails at runtime, they replace the hash algorithm the djb2 algorithm, which is way weaker, but works for their case.

An example adapted for our case may be:

function djb2Hash(data) {
    const chars = data.split("").map(str => str.charCodeAt(0));
    return `${chars.reduce((prev, curr) => ((prev << 5) + prev) + curr, 5381)}`;
}

console.log(Buffer.from(djb2Hash('This is a 😎 test of the djb2 hash function')).toString('hex'));
// prints 36373536373437333033

I am not asking you to do this, but the djb2 story is fun: see here, and the original mailing list post by Daniel Bernstein from 1991. He was 20 at the time.

[muxator]

Follow up which uses djb2 when there is no crypto support: #3797.