github-notifier: Restore MAX_BODY_LENGTH and raise from 200 kB to 1 MiB

github-notifier.js

@@ -2,6 +2,8 @@ const crypto = require('crypto');
 const util = require('util');
 const EventEmitter2 = require('eventemitter2').EventEmitter2;
 
+const MAX_BODY_LENGTH = 1024 * 1024; // 1 MiB
+
 function Notifier (config = {}, allowInsecure = false) {
   EventEmitter2.call(this, {
     wildcard: true,
@@ -41,10 +43,25 @@ Notifier.prototype.handler = function (request, response) {
   let body = '';
   request.on('data', function onData (chunk) {
     body += chunk;
+
+    if (body.length > MAX_BODY_LENGTH) {
+      body = '';
+      // SECURITY: Prevent crash/DOS through OOM by limiting the body buffer we receive.
+      //
+      // If you need to raise this, remember to also raise Nginx client_max_body_size.
+      //
+      // HTTP 413 Payload Too Large
+      response.writeHead(413);
+      response.end();
+
+      request.off('data', onData);
+      request.destroy();
+    }
   });
 
   request.on('end', function onEnd () {
     // Accept the request and close the connection
+    //
     // SECURITY: We decide on and close the response regardless of,
     // and prior to, any secret-based signature validation, so as to not
     // expose details about the outcome or timing of it to external clients.

test/server.js

@@ -349,4 +349,25 @@ cccccccc: Done!`
       assert.strictEqual(resp.statusCode, 202, 'status code');
     }, 500);
   });
+
+  QUnit.test('notifier ignores too large json', async assert => {
+    let called = 0;
+    function subscriber (notifier) {
+      notifier.on('example/test/push/heads/main', function () {
+        called++;
+      });
+    }
+    util.writeExportedJs(tmpDir, 'example.js', subscriber);
+
+    const server = await startServer({ insecure: true });
+    const address = `http://localhost:${server.address().port}`;
+    const resp = await util.request(address, util.mocks.badLargeJson);
+
+    const done = assert.async();
+    setTimeout(() => {
+      done();
+      assert.strictEqual(called, 0);
+      assert.strictEqual(resp.statusCode, 413, 'status code');
+    }, 500);
+  });
 });

test/util.js

@@ -163,6 +163,19 @@ module.exports = {
         deleted: false
       }
     },
+    badLargeJson: {
+      method: 'POST',
+      headers: {
+        'content-type': 'application/json',
+        'X-GitHub-Event': 'push'
+      },
+      body: {
+        a: 'x'.repeat(512 * 1024),
+        b: 'x'.repeat(512 * 1024),
+        c: 'x'.repeat(512 * 1024),
+        d: 'x'.repeat(512 * 1024)
+      }
+    },
     badInvalidJson: {
       method: 'POST',
       headers: {