Skip to content

Commit

Permalink
fix codeql 0217
Browse files Browse the repository at this point in the history
  • Loading branch information
YingXue committed Feb 6, 2025
1 parent 1388986 commit 6bc0a6c
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 18 deletions.
10 changes: 8 additions & 2 deletions src/server/serverBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import fetch from 'node-fetch';
import { EventHubConsumerClient, Subscription, ReceivedEventData, earliestEventPosition } from '@azure/event-hubs';
import { generateDataPlaneRequestBody, generateDataPlaneResponse } from './dataPlaneHelper';
import { convertIotHubToEventHubsConnectionString } from './eventHubHelper';
import { fetchDirectories, fetchDrivesOnWindows, findMatchingFile, readFileFromLocal } from './utils';
import { fetchDirectories, fetchDrivesOnWindows, findMatchingFile, isSafeUrl, readFileFromLocal } from './utils';

export const SERVER_ERROR = 500;
export const SUCCESS = 200;
Expand Down Expand Up @@ -203,8 +203,14 @@ export const handleModelRepoPostRequest = async (req: express.Request, res: expr
res.status(BAD_REQUEST).send();
}
const controllerRequest = req.body;
const userUri = controllerRequest.uri;
if (!(await isSafeUrl(userUri))) {
return res.status(403).send({ error: "Forbidden: Unsafe URL." });
}


try {
const response = await fetch(controllerRequest.uri,
const response = await fetch(userUri,
{
body: controllerRequest.body || null,
headers: controllerRequest.headers || null,
Expand Down
93 changes: 77 additions & 16 deletions src/server/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import express = require('express');
import * as fs from 'fs';
import * as path from 'path';
import * as dns from 'dns';
import { SERVER_ERROR, SUCCESS } from './serverBase';

export const fetchDrivesOnWindows = (res: express.Response) => {
Expand All @@ -16,26 +17,51 @@ export const fetchDrivesOnWindows = (res: express.Response) => {
};

export const fetchDirectories = (dir: string, res: express.Response) => {
const result: string[] = [];
const resolvedPath = fs.realpathSync(dir, { encoding: "utf8" });

for (const item of fs.readdirSync(resolvedPath)) {
try {
const root = path.resolve(dir);
const filePath = fs.realpathSync(path.resolve(dir, item));

if (filePath.startsWith(root)) {
const stat = fs.statSync(filePath);
if (stat.isDirectory()) {
result.push(item);
try {
if (!dir || typeof dir !== 'string') {
return res.status(400).send({ error: 'Invalid directory path.' });
}

// Ensure dir is an absolute path
if (!path.isAbsolute(dir)) {
return res.status(400).send({ error: 'Path must be absolute.' });
}

// Normalize and resolve symlinks
const resolvedPath = fs.realpathSync(dir);

// Prevent path traversal by ensuring it doesn’t contain `..`
if (resolvedPath.includes('..')) {
return res.status(400).send({ error: 'Path traversal detected.' });
}

// Ensure the path exists and is a directory
if (!fs.existsSync(resolvedPath) || !fs.statSync(resolvedPath).isDirectory()) {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
return res.status(400).send({ error: 'Directory does not exist.' });
}

const result: string[] = [];

for (const item of fs.readdirSync(resolvedPath)) {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
try {
const filePath = fs.realpathSync(path.join(resolvedPath, item));

// Ensure filePath is still within resolvedPath
if (filePath.startsWith(resolvedPath)) {
const stat = fs.statSync(filePath);
if (stat.isDirectory()) {
result.push(item);
}
}
} catch {
// Swallow error and continue
}
}
catch {
// some item cannot be checked by isDirectory(), swallow error and continue the loop
}

res.status(200).send(result);

Check failure

Code scanning / CodeQL

Stored cross-site scripting High

Stored cross-site scripting vulnerability due to
stored value
.
} catch {
res.status(500).send({ error: 'Failed to fetch directories.' });
}
res.status(SUCCESS).send(result);
};

// tslint:disable-next-line:cyclomatic-complexity
Expand Down Expand Up @@ -80,3 +106,38 @@ const isFileExtensionJson = (fileName: string) => {
export const readFileFromLocal = (filePath: string, fileName: string) => {
return fs.readFileSync(`${filePath}/${fileName}`, 'utf-8');
}

export const isSafeUrl = async (userUrl: string): Promise<boolean> => {
try {
const parsedUrl = new URL(userUrl);

// Enforce HTTPS
if (parsedUrl.protocol !== 'https:' && parsedUrl.protocol !== 'http:') {
return false;
}

// Resolve DNS to check IP addresses
const addresses = await dns.promises.resolve(parsedUrl.hostname);

// Block local and private IPs
for (const address of addresses) {
if (
address.startsWith('127.') || // Loopback
address.startsWith('10.') || // Private
address.startsWith('192.168.') || // Private
address.startsWith('169.254.') || // Link-local
address === '0.0.0.0' || // Unspecified
address === '::1' || // IPv6 loopback
address.startsWith('fe80:') || // IPv6 link-local
address.startsWith('fc00:') || // IPv6 unique local
address.startsWith('fd00:') // IPv6 unique local
) {
return false;
}
}

return true;
} catch {
return false;
}
};

0 comments on commit 6bc0a6c

Please sign in to comment.