CVE-2023-25653: Loop with Unreachable Exit Condition ('Infinite Loop') (CWE-835)
value: node-jose is a JavaScript implementation of the JSON Object Signing and Encryption (JOSE) for web browsers and node.js-based servers. Prior to version 2.2.0, when using the non-default "fallback" crypto back-end, ECC operations in `node-jose` can trigger a Denial-of-Service (DoS) condition, due to a possible infinite loop in an internal calculation.  For some ECC operations, this condition is triggered randomly; for others, it can be triggered by malicious input. The issue has been patched in version 2.2.0. Since this issue is only present in the "fallback" crypto implementation, it can be avoided by ensuring that either WebCrypto or the Node `crypto` module is available in the JS environment where `node-jose` is being run.
CWE description: The product contains an iteration or loop with an exit condition that cannot be reached, i.e., an infinite loop.
CVSS v3 base score: 7.5
Published: 2023-02-16T19:15Z
Repository: https://github.com/cisco/node-jose
Language: JavaScript
Files changed: lib/deps/ecc/math.js, lib/deps/forge.js, test/algorithms/ecc-test.js
Commit date: 2023-02-08 13:28:24 +0000
Commit message: Merge pull request from GHSA-5h4j-qrvg-9xhw


Diff: diff --git a/lib/deps/ecc/math.js b/lib/deps/ecc/math.js
index df9575c..15ec91c 100644
--- a/lib/deps/ecc/math.js
+++ b/lib/deps/ecc/math.js
@@ -44,10 +44,11 @@ function barrettConvert(x) {
 
 function barrettRevert(x) { return x; }
 
 // x = x mod m (HAC 14.42)
 function barrettReduce(x) {
+  if (x.s < 0) { throw Error("Barrett reduction on negative input"); }
   x.drShiftTo(this.m.t-1,this.r2);
   if(x.t > this.m.t+1) { x.t = this.m.t+1; x.clamp(); }
   this.mu.multiplyUpperTo(this.r2,this.m.t+1,this.q3);
   this.m.multiplyLowerTo(this.q3,this.m.t+1,this.r2);
   while(x.compareTo(this.r2) < 0) x.dAddOffset(1,this.m.t+1);
diff --git a/lib/deps/forge.js b/lib/deps/forge.js
index 5a7abde..3e898f5 100644
--- a/lib/deps/forge.js
+++ b/lib/deps/forge.js
@@ -86,6 +86,15 @@ modeRaw.prototype.decrypt = function(input, output, finish) {
       factory;
   factory = function() { return new forge.aes.Algorithm(name, mode); };
   forge.cipher.registerAlgorithm(name, factory);
 })();
 
+// Ensure that the jsbn modInverse function always returns a positive result
+const originalModInverse = forge.jsbn.BigInteger.prototype.modInverse;
+const positiveModInverse = function(m) {
+  const inv = originalModInverse.apply(this, [m]);
+  return inv.mod(m);
+}
+
+forge.jsbn.BigInteger.prototype.modInverse = positiveModInverse;
+
 module.exports = forge;
diff --git a/test/algorithms/ecc-test.js b/test/algorithms/ecc-test.js
new file mode 100644
index 0000000..18a99ce
--- /dev/null
+++ b/test/algorithms/ecc-test.js
@@ -0,0 +1,29 @@
+/*!
+ *
+ * Copyright (c) 2015 Cisco Systems, Inc. See LICENSE file.
+ */
+"use strict";
+
+var assert = require("chai").assert;
+
+const CURVES = require('../../lib/deps/ecc/curves.js');
+const BigInteger = require('../../lib/deps/forge').jsbn.BigInteger;
+
+describe("ecc/positive", function() {
+  const negativeModInverseCases = [
+    '101067240514044546216936289506154965497874315269115226505131909313278720169941',
+    '47260992668897782856940293132731814279826643476197468731642996160637470667669',
+  ]
+  
+  const p = CURVES["P-256"].curve.p;
+
+  const runner = () => {
+    for (const kStr of negativeModInverseCases) {
+      const k = new BigInteger(kStr);
+      const kinv = k.modInverse(p);
+      assert.isAtLeast(kinv.s, 0, "Negative mod inverse");
+    }
+  };
+
+  it('normalizes negative modular inverses', runner);
+})

Fixed code:   if (x.s < 0) { throw Error("Barrett reduction on negative input"); }
// Ensure that the jsbn modInverse function always returns a positive result
const originalModInverse = forge.jsbn.BigInteger.prototype.modInverse;
const positiveModInverse = function(m) {
  const inv = originalModInverse.apply(this, [m]);
  return inv.mod(m);
}

forge.jsbn.BigInteger.prototype.modInverse = positiveModInverse;

/*!
 *
 * Copyright (c) 2015 Cisco Systems, Inc. See LICENSE file.
 */
"use strict";

var assert = require("chai").assert;

const CURVES = require('../../lib/deps/ecc/curves.js');
const BigInteger = require('../../lib/deps/forge').jsbn.BigInteger;

describe("ecc/positive", function() {
  const negativeModInverseCases = [
    '101067240514044546216936289506154965497874315269115226505131909313278720169941',
    '47260992668897782856940293132731814279826643476197468731642996160637470667669',
  ]
  
  const p = CURVES["P-256"].curve.p;

  const runner = () => {
    for (const kStr of negativeModInverseCases) {
      const k = new BigInteger(kStr);
      const kinv = k.modInverse(p);
      assert.isAtLeast(kinv.s, 0, "Negative mod inverse");
    }
  };

  it('normalizes negative modular inverses', runner);
})