Merge pull request #339 from AikidoSec/fix-legacy-url

Fix legacy url object in http request

library/sinks/HTTPRequest.test.ts

@@ -285,6 +285,17 @@ t.test("it works", (t) => {
         "Aikido firewall has blocked a server-side request forgery: https.request(...) originating from body.image"
       );
     }
+
+    const oldUrl = require("url");
+    const error6 = t.throws(() =>
+      https.request(oldUrl.parse("https://localhost:4000/api/internal"))
+    );
+    if (error6 instanceof Error) {
+      t.same(
+        error6.message,
+        "Aikido firewall has blocked a server-side request forgery: https.request(...) originating from body.image"
+      );
+    }
   });
 
   runWithContext(

library/sinks/HTTPRequest.ts

@@ -6,12 +6,12 @@ import { Hooks } from "../agent/hooks/Hooks";
 import { InterceptorResult } from "../agent/hooks/MethodInterceptor";
 import { Wrapper } from "../agent/Wrapper";
 import { getPortFromURL } from "../helpers/getPortFromURL";
-import { isPlainObject } from "../helpers/isPlainObject";
 import { checkContextForSSRF } from "../vulnerabilities/ssrf/checkContextForSSRF";
 import { inspectDNSLookupCalls } from "../vulnerabilities/ssrf/inspectDNSLookupCalls";
 import { isRedirectToPrivateIP } from "../vulnerabilities/ssrf/isRedirectToPrivateIP";
 import { getUrlFromHTTPRequestArgs } from "./http-request/getUrlFromHTTPRequestArgs";
 import { wrapResponseHandler } from "./http-request/wrapResponseHandler";
+import { isOptionsObject } from "./http-request/isOptionsObject";
 
 export class HTTPRequest implements Wrapper {
   private inspectHostname(
@@ -98,7 +98,7 @@ export class HTTPRequest implements Wrapper {
     }
 
     const optionObj = args.find((arg): arg is RequestOptions =>
-      isPlainObject(arg)
+      isOptionsObject(arg)
     );
 
     const url = getUrlFromHTTPRequestArgs(args, module);

library/sinks/Undici.test.ts

@@ -60,7 +60,7 @@ t.test(
     skip:
       getMajorNodeVersion() <= 16 ? "ReadableStream is not available" : false,
   },
-  async () => {
+  async (t) => {
     const logger = new LoggerForTesting();
     const agent = new Agent(
       true,
@@ -193,6 +193,17 @@ t.test(
           "Aikido firewall has blocked a server-side request forgery: undici.fetch(...) originating from body.image"
         );
       }
+
+      const oldUrl = require("url");
+      const error5 = t.throws(() =>
+        request(oldUrl.parse("https://localhost:4000/api/internal"))
+      );
+      if (error5 instanceof Error) {
+        t.same(
+          error5.message,
+          "Aikido firewall has blocked a server-side request forgery: undici.request(...) originating from body.image"
+        );
+      }
     });
 
     await runWithContext(

library/sinks/Undici.ts

@@ -9,11 +9,11 @@ import {
   getMinorNodeVersion,
 } from "../helpers/getNodeVersion";
 import { getPortFromURL } from "../helpers/getPortFromURL";
-import { isPlainObject } from "../helpers/isPlainObject";
 import { tryParseURL } from "../helpers/tryParseURL";
 import { checkContextForSSRF } from "../vulnerabilities/ssrf/checkContextForSSRF";
 import { inspectDNSLookupCalls } from "../vulnerabilities/ssrf/inspectDNSLookupCalls";
 import { wrapDispatch } from "./undici/wrapDispatch";
+import { isOptionsObject } from "./http-request/isOptionsObject";
 
 const methods = [
   "request",
@@ -104,7 +104,7 @@ export class Undici implements Wrapper {
       }
 
       if (
-        isPlainObject(args[0]) &&
+        isOptionsObject(args[0]) &&
         typeof args[0].hostname === "string" &&
         args[0].hostname.length > 0
       ) {

library/sinks/http-request/getUrlFromHTTPRequestArgs.test.ts

@@ -125,3 +125,12 @@ t.test("Pass host instead of hostname", async (t) => {
     new URL("https://test.dev")
   );
 });
+
+t.test("it works with node:url object as first argument", async (t) => {
+  const oldUrl = require("url");
+
+  t.same(
+    getURL([oldUrl.parse("http://localhost:4000")], "http"),
+    new URL("http://localhost:4000")
+  );
+});

library/sinks/http-request/getUrlFromHTTPRequestArgs.ts

@@ -1,7 +1,7 @@
 import type { RequestOptions as HTTPSRequestOptions } from "https";
 import type { RequestOptions as HTTPRequestOptions } from "http";
 import { tryParseURL } from "../../helpers/tryParseURL";
-import { isPlainObject } from "../../helpers/isPlainObject";
+import { isOptionsObject } from "./isOptionsObject";
 
 /**
  * Gets the url from the arguments of an node:http(s) outgoing request function call.
@@ -43,11 +43,11 @@ export function getUrlFromHTTPRequestArgs(
  * But thy can also be not provided at all.
  */
 function getRequestOptions(args: unknown[]) {
-  if (isPlainObject(args[0]) && !(args[0] instanceof URL)) {
+  if (isOptionsObject(args[0]) && !(args[0] instanceof URL)) {
     return args[0] as HTTPRequestOptions | HTTPSRequestOptions;
   } else if (
     args.length > 1 &&
-    isPlainObject(args[1]) &&
+    isOptionsObject(args[1]) &&
     !(args[1] instanceof URL)
   ) {
     return args[1] as HTTPRequestOptions | HTTPSRequestOptions;
@@ -74,6 +74,7 @@ function getUrlFromRequestOptions(
   } else if (typeof options.host === "string") {
     str += options.host;
   }
+
   if (options.port) {
     if (typeof options.port === "number" && options.port > 0) {
       str += `:${options.port}`;

library/sinks/http-request/isOptionsObject.test.ts

@@ -0,0 +1,27 @@
+import * as t from "tap";
+import { isOptionsObject } from "./isOptionsObject";
+
+t.test("it works with objects", async (t) => {
+  t.equal(isOptionsObject({}), true);
+  t.equal(isOptionsObject({ protocol: "https:" }), true);
+  t.equal(isOptionsObject({ hostname: "localhost" }), true);
+  t.equal(isOptionsObject({ port: 4000 }), true);
+  t.equal(isOptionsObject({ path: "/test" }), true);
+  t.equal(
+    isOptionsObject({
+      protocol: "https:",
+      hostname: "localhost",
+      port: 4000,
+      path: "/test",
+    }),
+    true
+  );
+});
+
+t.test("it works with non-objects", async (t) => {
+  t.equal(isOptionsObject("test"), false);
+  t.equal(isOptionsObject(1), false);
+  t.equal(isOptionsObject([]), false);
+  t.equal(isOptionsObject(null), false);
+  t.equal(isOptionsObject(undefined), false);
+});

library/sinks/http-request/isOptionsObject.ts

@@ -0,0 +1,7 @@
+/**
+ * Check if the argument is treated as an options object by Node.js.
+ * For checking if the argument can be used as options for a outgoing HTTP request.
+ */
+export function isOptionsObject(arg: any): arg is { [key: string]: unknown } {
+  return typeof arg === "object" && !Array.isArray(arg) && arg !== null;
+}