http2: treat non-EOF empty frames like other invalid frames

Use the existing mechanism that we have to keep track of invalid frames for treating this specific kind of invalid frame. The commit that originally introduced this check was 695e38b, which was supposed to proected against CVE-2019-9518, which in turn was specifically about a *flood* of empty data frames. While these are still invalid frames either way, it makes sense to be forgiving here and just treat them like other invalid frames, i.e. to allow a small (configurable) number of them. Fixes: #37849 PR-URL: #37875 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>

Author: addaleax

src/node_http2.cc

@@ -1285,7 +1285,11 @@ int Http2Session::HandleDataFrame(const nghttp2_frame* frame){
  frame->hd.flags & NGHTTP2_FLAG_END_STREAM){
  stream->EmitRead(UV_EOF);
  } elseif (frame->hd.length == 0){
-return1; // Consider 0-length frame without END_STREAM an error.
+if (invalid_frame_count_++ > js_fields_->max_invalid_frames){
+Debug(this, "rejecting empty-frame-without-END_STREAM flood\n");
+// Consider a flood of 0-length frames without END_STREAM an error.
+return1;
+ }
  }
 return0;
 }

test/fixtures/emptyframe.http2

@@ -0,0 +1,39 @@
+'use strict';
+constcommon=require('../common');
+if(!common.hasCrypto)
+common.skip('missing crypto');
+const{ readSync }=require('../common/fixtures');
+constnet=require('net');
+consthttp2=require('http2');
+const{ once }=require('events');
+
+asyncfunctionmain(){
+constblobWithEmptyFrame=readSync('emptyframe.http2');
+constserver=net.createServer((socket)=>{
+socket.end(blobWithEmptyFrame);
+}).listen(0);
+awaitonce(server,'listening');
+
+for(constmaxSessionInvalidFramesof[0,2]){
+constclient=http2.connect(`http://localhost:${server.address().port}`,{
+ maxSessionInvalidFrames
+});
+conststream=client.request({
+':method': 'GET',
+':path': '/'
+});
+if(maxSessionInvalidFrames){
+stream.on('error',common.mustNotCall());
+client.on('error',common.mustNotCall());
+}else{
+stream.on('error',common.mustCall());
+client.on('error',common.mustCall());
+}
+stream.resume();
+awaitonce(stream,'end');
+client.close();
+}
+server.close();
+}
+
+main().then(common.mustCall());