From 811f5500dbc53d16dddb64adc52deca7cfc9944f Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Thu, 28 May 2026 00:46:12 +0200 Subject: [PATCH] THRIFT-6046: Harden JavaScript protocol recursion depth for struct read/write Client: js Co-Authored-By: Claude Sonnet 4.6 --- .../cpp/src/thrift/generate/t_js_generator.cc | 22 +++++++ lib/js/src/thrift.js | 18 ++++++ lib/js/test/test-skip-depth.js | 38 +++++++++++ lib/nodejs/lib/thrift/binary_protocol.js | 16 +++++ lib/nodejs/lib/thrift/compact_protocol.js | 16 +++++ lib/nodejs/lib/thrift/json_protocol.js | 16 +++++ lib/nodejs/lib/thrift/thrift.js | 2 + lib/nodejs/test/recursion_depth.test.js | 64 +++++++++++++++++++ 8 files changed, 192 insertions(+) create mode 100644 lib/nodejs/test/recursion_depth.test.js diff --git a/compiler/cpp/src/thrift/generate/t_js_generator.cc b/compiler/cpp/src/thrift/generate/t_js_generator.cc index a7ee220282f..c869be74d32 100644 --- a/compiler/cpp/src/thrift/generate/t_js_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_js_generator.cc @@ -1142,6 +1142,10 @@ void t_js_generator::generate_js_struct_reader(ostream& out, t_struct* tstruct) indent_up(); + indent(out) << "input.incrementRecursionDepth();" << '\n'; + indent(out) << "try {" << '\n'; + indent_up(); + indent(out) << "input.readStructBegin();" << '\n'; // Loop over reading in fields @@ -1204,6 +1208,13 @@ void t_js_generator::generate_js_struct_reader(ostream& out, t_struct* tstruct) indent(out) << "input.readStructEnd();" << '\n'; + indent_down(); + indent(out) << "} finally {" << '\n'; + indent_up(); + indent(out) << "input.decrementRecursionDepth();" << '\n'; + indent_down(); + indent(out) << "}" << '\n'; + indent(out) << "return;" << '\n'; indent_down(); @@ -1232,6 +1243,10 @@ void t_js_generator::generate_js_struct_writer(ostream& out, t_struct* tstruct) indent_up(); + indent(out) << "output.incrementRecursionDepth();" << '\n'; + indent(out) << "try {" << '\n'; + indent_up(); + indent(out) << "output.writeStructBegin('" << name << "');" << '\n'; for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) { @@ -1255,6 +1270,13 @@ void t_js_generator::generate_js_struct_writer(ostream& out, t_struct* tstruct) out << indent() << "output.writeFieldStop();" << '\n' << indent() << "output.writeStructEnd();" << '\n'; + indent_down(); + out << indent() << "} finally {" << '\n'; + indent_up(); + out << indent() << "output.decrementRecursionDepth();" << '\n'; + indent_down(); + out << indent() << "}" << '\n'; + out << indent() << "return;" << '\n'; indent_down(); diff --git a/lib/js/src/thrift.js b/lib/js/src/thrift.js index 20078f54fa6..68f18353e66 100644 --- a/lib/js/src/thrift.js +++ b/lib/js/src/thrift.js @@ -282,6 +282,8 @@ Thrift.TProtocolExceptionType = { DEPTH_LIMIT: 6 }; +Thrift.DEFAULT_RECURSION_DEPTH = 64; + Thrift.TProtocolException = function TProtocolException(type, message) { Error.call(this); if (Error.captureStackTrace !== undefined) { @@ -737,6 +739,7 @@ Thrift.TJSONProtocol = Thrift.Protocol = function(transport) { this.tstack = []; this.tpos = []; this.transport = transport; + this._recursion_depth = 0; }; /** @@ -1517,6 +1520,21 @@ Thrift.Protocol.prototype = { } }; +Thrift.Protocol.prototype.incrementRecursionDepth = function() { + this._recursion_depth += 1; + if (this._recursion_depth > Thrift.DEFAULT_RECURSION_DEPTH) { + this._recursion_depth -= 1; + throw new Thrift.TProtocolException( + Thrift.TProtocolExceptionType.DEPTH_LIMIT, + 'Maximum recursion depth exceeded' + ); + } +}; + +Thrift.Protocol.prototype.decrementRecursionDepth = function() { + this._recursion_depth -= 1; +}; + /** * Initializes a MutilplexProtocol Implementation as a Wrapper for Thrift.Protocol diff --git a/lib/js/test/test-skip-depth.js b/lib/js/test/test-skip-depth.js index 212fc4eee13..7ddf854de77 100644 --- a/lib/js/test/test-skip-depth.js +++ b/lib/js/test/test-skip-depth.js @@ -43,3 +43,41 @@ QUnit.test('skip does not throw below the depth limit', function(assert) { assert.ok(protocol.skip(Thrift.Type.I32, 63) !== undefined || true, 'skip at depth 63 does not throw'); }); + +QUnit.module('Protocol recursion depth limit'); + +QUnit.test('incrementRecursionDepth allows up to DEFAULT_RECURSION_DEPTH levels', function(assert) { + var transport = new Thrift.Transport('/service'); + var protocol = new Thrift.Protocol(transport); + for (var i = 0; i < Thrift.DEFAULT_RECURSION_DEPTH; i++) { + assert.ok((function() { protocol.incrementRecursionDepth(); return true; })(), + 'depth ' + (i + 1) + ' should not throw'); + } +}); + +QUnit.test('incrementRecursionDepth throws DEPTH_LIMIT when depth exceeds DEFAULT_RECURSION_DEPTH', function(assert) { + var transport = new Thrift.Transport('/service'); + var protocol = new Thrift.Protocol(transport); + for (var i = 0; i < Thrift.DEFAULT_RECURSION_DEPTH; i++) { + protocol.incrementRecursionDepth(); + } + assert.throws( + function() { protocol.incrementRecursionDepth(); }, + function(e) { + return e instanceof Thrift.TProtocolException && + e.type === Thrift.TProtocolExceptionType.DEPTH_LIMIT; + }, + 'throws TProtocolException with DEPTH_LIMIT at depth ' + (Thrift.DEFAULT_RECURSION_DEPTH + 1) + ); +}); + +QUnit.test('decrementRecursionDepth restores capacity', function(assert) { + var transport = new Thrift.Transport('/service'); + var protocol = new Thrift.Protocol(transport); + for (var i = 0; i < Thrift.DEFAULT_RECURSION_DEPTH; i++) { + protocol.incrementRecursionDepth(); + } + protocol.decrementRecursionDepth(); + assert.ok((function() { protocol.incrementRecursionDepth(); return true; })(), + 'after decrement, one more increment should succeed'); +}); diff --git a/lib/nodejs/lib/thrift/binary_protocol.js b/lib/nodejs/lib/thrift/binary_protocol.js index b2c3720a6b6..9911d637d3d 100644 --- a/lib/nodejs/lib/thrift/binary_protocol.js +++ b/lib/nodejs/lib/thrift/binary_protocol.js @@ -44,6 +44,7 @@ function TBinaryProtocol(trans, strictRead, strictWrite) { this.strictRead = strictRead !== undefined ? strictRead : false; this.strictWrite = strictWrite !== undefined ? strictWrite : true; this._seqid = null; + this._recursion_depth = 0; } TBinaryProtocol.prototype.flush = function () { @@ -388,3 +389,18 @@ TBinaryProtocol.prototype.skip = function (type, depth) { throw new Error("Invalid type: " + type); } }; + +TBinaryProtocol.prototype.incrementRecursionDepth = function () { + this._recursion_depth += 1; + if (this._recursion_depth > Thrift.DEFAULT_RECURSION_DEPTH) { + this._recursion_depth -= 1; + throw new Thrift.TProtocolException( + Thrift.TProtocolExceptionType.DEPTH_LIMIT, + "Maximum recursion depth exceeded" + ); + } +}; + +TBinaryProtocol.prototype.decrementRecursionDepth = function () { + this._recursion_depth -= 1; +}; diff --git a/lib/nodejs/lib/thrift/compact_protocol.js b/lib/nodejs/lib/thrift/compact_protocol.js index 2f6fc71b4cc..b1040c0e977 100644 --- a/lib/nodejs/lib/thrift/compact_protocol.js +++ b/lib/nodejs/lib/thrift/compact_protocol.js @@ -58,6 +58,7 @@ function TCompactProtocol(trans) { hasBoolValue: false, boolValue: false, }; + this._recursion_depth = 0; } // @@ -901,6 +902,21 @@ TCompactProtocol.prototype.zigzagToI64 = function (n) { return new Int64(hi, lo); }; +TCompactProtocol.prototype.incrementRecursionDepth = function () { + this._recursion_depth += 1; + if (this._recursion_depth > Thrift.DEFAULT_RECURSION_DEPTH) { + this._recursion_depth -= 1; + throw new Thrift.TProtocolException( + Thrift.TProtocolExceptionType.DEPTH_LIMIT, + "Maximum recursion depth exceeded" + ); + } +}; + +TCompactProtocol.prototype.decrementRecursionDepth = function () { + this._recursion_depth -= 1; +}; + TCompactProtocol.prototype.skip = function (type, depth) { depth = (depth || 0) + 1; if (depth > 64) { diff --git a/lib/nodejs/lib/thrift/json_protocol.js b/lib/nodejs/lib/thrift/json_protocol.js index f8dd7e60deb..60daf5d322b 100644 --- a/lib/nodejs/lib/thrift/json_protocol.js +++ b/lib/nodejs/lib/thrift/json_protocol.js @@ -43,6 +43,7 @@ function TJSONProtocol(trans) { this.tstack = []; this.tpos = []; this.trans = trans; + this._recursion_depth = 0; } /** @@ -781,6 +782,21 @@ TJSONProtocol.prototype.getTransport = function () { /** * Method to arbitrarily skip over data */ +TJSONProtocol.prototype.incrementRecursionDepth = function () { + this._recursion_depth += 1; + if (this._recursion_depth > Thrift.DEFAULT_RECURSION_DEPTH) { + this._recursion_depth -= 1; + throw new Thrift.TProtocolException( + Thrift.TProtocolExceptionType.DEPTH_LIMIT, + "Maximum recursion depth exceeded" + ); + } +}; + +TJSONProtocol.prototype.decrementRecursionDepth = function () { + this._recursion_depth -= 1; +}; + TJSONProtocol.prototype.skip = function (type, depth) { depth = (depth || 0) + 1; if (depth > 64) { diff --git a/lib/nodejs/lib/thrift/thrift.js b/lib/nodejs/lib/thrift/thrift.js index 698f9c93a4e..af0a7f053dc 100644 --- a/lib/nodejs/lib/thrift/thrift.js +++ b/lib/nodejs/lib/thrift/thrift.js @@ -150,6 +150,8 @@ var TProtocolExceptionType = (exports.TProtocolExceptionType = { DEPTH_LIMIT: 6, }); +exports.DEFAULT_RECURSION_DEPTH = 64; + exports.TProtocolException = TProtocolException; function TProtocolException(type, message) { diff --git a/lib/nodejs/test/recursion_depth.test.js b/lib/nodejs/test/recursion_depth.test.js new file mode 100644 index 00000000000..7d3cbf58224 --- /dev/null +++ b/lib/nodejs/test/recursion_depth.test.js @@ -0,0 +1,64 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +const test = require("tape"); +const Thrift = require("thrift/lib/nodejs/lib/thrift/thrift"); +const TBinaryProtocol = require("thrift/lib/nodejs/lib/thrift/binary_protocol"); +const TCompactProtocol = require("thrift/lib/nodejs/lib/thrift/compact_protocol"); +const TJSONProtocol = require("thrift/lib/nodejs/lib/thrift/json_protocol"); + +function makeProtocol(Proto) { + return new Proto({ flush: function() {} }); +} + +["TBinaryProtocol", "TCompactProtocol", "TJSONProtocol"].forEach(function(name) { + const Proto = { TBinaryProtocol, TCompactProtocol, TJSONProtocol }[name]; + + test(name + ": incrementRecursionDepth allows up to DEFAULT_RECURSION_DEPTH levels", function(assert) { + const p = makeProtocol(Proto); + for (let i = 0; i < Thrift.DEFAULT_RECURSION_DEPTH; i++) { + assert.doesNotThrow(function() { p.incrementRecursionDepth(); }, + "depth " + (i + 1) + " should not throw"); + } + assert.end(); + }); + + test(name + ": incrementRecursionDepth throws DEPTH_LIMIT at depth " + (Thrift.DEFAULT_RECURSION_DEPTH + 1), function(assert) { + const p = makeProtocol(Proto); + for (let i = 0; i < Thrift.DEFAULT_RECURSION_DEPTH; i++) { + p.incrementRecursionDepth(); + } + let caught = null; + try { p.incrementRecursionDepth(); } catch (e) { caught = e; } + assert.ok(caught instanceof Thrift.TProtocolException, "throws TProtocolException"); + assert.equal(caught && caught.type, Thrift.TProtocolExceptionType.DEPTH_LIMIT, "type is DEPTH_LIMIT"); + assert.end(); + }); + + test(name + ": decrementRecursionDepth restores capacity", function(assert) { + const p = makeProtocol(Proto); + for (let i = 0; i < Thrift.DEFAULT_RECURSION_DEPTH; i++) { + p.incrementRecursionDepth(); + } + p.decrementRecursionDepth(); + assert.doesNotThrow(function() { p.incrementRecursionDepth(); }, + "after decrement, one more increment should succeed"); + assert.end(); + }); +});