security: replace vulnerable parse-duration with CJS compatible ms library (#960)

* security: replace vulnerable parse-duration with CJS compatible ms library

* add comment about duration format

Author: aabmass

package.json

@@ -36,7 +36,7 @@
     "delay": "^5.0.0",
     "extend": "^3.0.2",
     "gcp-metadata": "^6.0.0",
-    "parse-duration": "^1.0.0",
+    "ms": "^2.1.3",
     "pprof": "4.0.0",
     "pretty-ms": "^7.0.0",
     "protobufjs": "~7.4.0",
@@ -47,6 +47,7 @@
     "@types/extend": "^3.0.0",
     "@types/long": "^5.0.0",
     "@types/mocha": "^9.0.0",
+    "@types/ms": "^2.1.0",
     "@types/nock": "^11.0.0",
     "@types/node": "^20.4.9",
     "@types/pretty-ms": "^5.0.0",

src/config.ts

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 import {GoogleAuthOptions} from '@google-cloud/common';
-import parseDuration from 'parse-duration';
+import * as ms from 'ms';
 
 // Configuration for Profiler.
 export interface Config extends GoogleAuthOptions {
@@ -185,7 +185,7 @@ export const defaultConfig = {
   heapMaxStackDepth: 64,
   ignoreHeapSamplesPath: '@google-cloud/profiler',
   initialBackoffMillis: 60 * 1000, // 1 minute
-  backoffCapMillis: parseDuration('1h')!,
+  backoffCapMillis: ms('1h')!,
   backoffMultiplier: 1.3,
   apiEndpoint: 'cloudprofiler.googleapis.com',
 

src/profiler.ts

@@ -29,7 +29,7 @@ import * as r from 'teeny-request';
 import {ProfilerConfig} from './config';
 import {createLogger} from './logger';
 
-import parseDuration from 'parse-duration';
+import * as ms from 'ms';
 // eslint-disable-next-line @typescript-eslint/no-var-requires
 const pjson = require('../../package.json');
 const SCOPE = 'https://www.googleapis.com/auth/monitoring.write';
@@ -111,7 +111,12 @@ function getServerResponseBackoff(body: object): number | undefined {
         item.retryDelay &&
         typeof item.retryDelay === 'string'
       ) {
-        const backoffMillis = parseDuration(item.retryDelay)!;
+        // item is a RetryInfo
+        // https://github.com/googleapis/googleapis/blob/4ec607bd375cddbec6d28bc1931eab7da221e4bb/google/rpc/error_details.proto#L92
+        // and the ProtoJSON encoding of the duration will be a string of seconds with "s"
+        // suffix https://protobuf.dev/programming-guides/json
+        const retryDelay: `${number}s` = item.retryDelay;
+        const backoffMillis = ms(retryDelay);
         if (backoffMillis > 0) {
           return backoffMillis;
         }
@@ -121,30 +126,6 @@ function getServerResponseBackoff(body: object): number | undefined {
   return undefined;
 }
 
-/**
- * @return if the backoff duration can be parsed, then the backoff duration in
- * ms, otherwise undefined.
- *
- * Public for testing.
- */
-export function parseBackoffDuration(
-  backoffMessage: string
-): number | undefined {
-  const backoffMessageRegex =
-    /action throttled, backoff for ((?:([0-9]+)h)?(?:([0-9]+)m)?([0-9.]+)s)$/;
-  const [, duration] = backoffMessageRegex.exec(backoffMessage) || [
-    undefined,
-    undefined,
-  ];
-  if (duration) {
-    const backoffMillis = parseDuration(duration)!;
-    if (backoffMillis > 0) {
-      return backoffMillis;
-    }
-  }
-  return undefined;
-}
-
 /**
  * @return true if an deployment is a Deployment and false otherwise.
  */
@@ -452,7 +433,7 @@ export class Profiler extends ServiceObject {
       // Default timeout for for a request is 1 minute, but request to create
       // profile is designed to hang until it is time to collect a profile
       // (up to one hour).
-      timeout: parseDuration('1h')!,
+      timeout: ms('1h')!,
     };
 
     this.logger.debug('Attempting to create profile.');
@@ -551,7 +532,7 @@ export class Profiler extends ServiceObject {
     if (prof.duration === undefined) {
       throw Error('Cannot collect time profile, duration is undefined.');
     }
-    const durationMillis = parseDuration(prof.duration);
+    const durationMillis = ms(prof.duration as ms.StringValue);
     if (!durationMillis) {
       throw Error(
         `Cannot collect time profile, duration "${prof.duration}" cannot` +

test/test-profiler.ts

@@ -28,12 +28,7 @@ import * as zlib from 'zlib';
 
 import {perftools} from 'pprof/proto/profile';
 import {ProfilerConfig} from '../src/config';
-import {
-  parseBackoffDuration,
-  Profiler,
-  Retryer,
-  BackoffResponseError,
-} from '../src/profiler';
+import {Profiler, Retryer, BackoffResponseError} from '../src/profiler';
 
 import {
   decodedHeapProfile,
@@ -42,7 +37,7 @@ import {
   timeProfile,
 } from './profiles-for-tests';
 
-import parseDuration from 'parse-duration';
+import * as ms from 'ms';
 // eslint-disable-next-line @typescript-eslint/no-var-requires
 const fakeCredentials = require('../../test/fixtures/gcloud-credentials.json');
 
@@ -66,9 +61,9 @@ const testConfig: ProfilerConfig = {
   heapMaxStackDepth: 64,
   ignoreHeapSamplesPath: '@google-cloud/profiler',
   initialBackoffMillis: 1000,
-  backoffCapMillis: parseDuration('1h')!,
+  backoffCapMillis: ms('1h')!,
   backoffMultiplier: 1.3,
-  serverBackoffCapMillis: parseDuration('7d')!,
+  serverBackoffCapMillis: ms('7d')!,
   localProfilingPeriodMillis: 1000,
   localTimeDurationMillis: 1000,
   localLogPeriodMillis: 1000,
@@ -891,7 +886,7 @@ describe('Profiler', () => {
           );
         const profiler = new Profiler(testConfig);
         const delayMillis = await profiler.collectProfile();
-        assert.strictEqual(parseDuration('7d'), delayMillis);
+        assert.strictEqual(ms('7d'), delayMillis);
       }
     );
     it(
@@ -919,48 +914,4 @@ describe('Profiler', () => {
       }
     );
   });
-  describe('parseBackoffDuration', () => {
-    it('should return undefined when no duration specified', () => {
-      assert.strictEqual(undefined, parseBackoffDuration(''));
-    });
-    it('should parse backoff with minutes and seconds specified', () => {
-      assert.strictEqual(
-        62000,
-        parseBackoffDuration('action throttled, backoff for 1m2s')
-      );
-    });
-    it('should parse backoff with fraction of second', () => {
-      assert.strictEqual(
-        2500,
-        parseBackoffDuration('action throttled, backoff for 2.5s')
-      );
-    });
-    it('should parse backoff with minutes and seconds, including fraction of second', () => {
-      assert.strictEqual(
-        62500,
-        parseBackoffDuration('action throttled, backoff for 1m2.5s')
-      );
-    });
-    it('should parse backoff with hours and seconds', () => {
-      assert.strictEqual(
-        3602500,
-        parseBackoffDuration('action throttled, backoff for 1h2.5s')
-      );
-    });
-    it('should parse backoff with hours, minutes, and seconds', () => {
-      assert.strictEqual(
-        3662500,
-        parseBackoffDuration('action throttled, backoff for 1h1m2.5s')
-      );
-    });
-    it('should parse return undefined for unexpected backoff time string format', () => {
-      assert.strictEqual(
-        undefined,
-        parseBackoffDuration('action throttled, backoff for  1m2+s')
-      );
-    });
-    it('should parse return undefined for unexpected string format', () => {
-      assert.strictEqual(undefined, parseBackoffDuration('time 1m2s'));
-    });
-  });
 });