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 7, 2025
1 parent 1388986 commit 7926c99
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 28 deletions.
8 changes: 8 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
"crypto-browserify": "3.12.0",
"date-fns": "2.14.0",
"electron-window-state": "5.0.3",
"escape-html": "^1.0.3",
"express": "4.21.2",
"i18next": "20.6.1",
"immutable": "4.0.0-rc.12",
Expand Down Expand Up @@ -114,6 +115,7 @@
"@types/cors": "2.8.4",
"@types/enzyme": "3.10.8",
"@types/enzyme-adapter-react-16": "1.0.5",
"@types/escape-html": "^1.0.4",
"@types/express": "4.16.0",
"@types/jest": "26.0.24",
"@types/jest-plugin-context": "2.9.0",
Expand Down Expand Up @@ -189,7 +191,9 @@
"transform": {
"^.+\\.(js|jsx|ts|tsx)$": "ts-jest"
},
"transformIgnorePatterns": ["node_modules/(?!react-movable)"],
"transformIgnorePatterns": [
"node_modules/(?!react-movable)"
],
"testRegex": "(\\.|/)(spec)\\.(tsx?)$",
"moduleNameMapper": {
"^.+\\.(scss)$": "<rootDir>/scss-stub.js",
Expand Down
6 changes: 3 additions & 3 deletions src/app/shared/components/monacoEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ export const MonacoEditorComponent: React.FC<MonacoEditorComponentProps> = ({
we will need to shift focus to another DOM element or window.
It is due to Electron's integration with Chromium and the way focus is managed between the webview and the main process.
We will create a dummy input, move the focus to it, and then clean it up. */
const dummyInput = document.createElement("input");
dummyInput.style.position = "absolute";
dummyInput.style.opacity = "0";
const dummyInput = document.createElement('input');
dummyInput.style.position = 'absolute';
dummyInput.style.opacity = '0';
document.body.appendChild(dummyInput);
dummyInput.focus();
setTimeout(() => document.body.removeChild(dummyInput), 0);
Expand Down
8 changes: 4 additions & 4 deletions src/server/serverBase.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ describe('serverBase', () => {
};

const mockResponse = () => {
const res = {status: undefined, json: undefined, send: undefined};
const res = {status: undefined, json: undefined, send: undefined} as any; // tslint:disable-line:no-any
res.status = jest.fn().mockReturnValue(res);
res.send = jest.fn().mockReturnValue(res);
res.json = jest.fn().mockReturnValue(res);
return res as any; // tslint:disable-line:no-any
return res as any; // tslint:disable-line:no-any
};

context('handleDataPlanePostRequest', () => {
Expand Down Expand Up @@ -47,10 +47,10 @@ describe('serverBase', () => {
});

it('returns 500 if response is error', async () => {
const req = mockRequest('test');
const req = mockRequest({url: 'test'});
const res = mockResponse();
await ServerBase.handleModelRepoPostRequest(req, res);
expect(res.status).toHaveBeenCalledWith(500); // tslint:disable-line:no-magic-numbers
expect(res.status).toHaveBeenCalledWith(403); // tslint:disable-line:no-magic-numbers
});
});
});
14 changes: 10 additions & 4 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 @@ -199,12 +199,18 @@ export const handleEventHubStopPostRequest = (req: express.Request, res: express

const modelRepoUri = '/api/ModelRepo';
export const handleModelRepoPostRequest = async (req: express.Request, res: express.Response) => {
if (!req.body) {
const controllerRequest = req.body;
const userUri = controllerRequest?.uri;
if (!controllerRequest || !userUri) {
res.status(BAD_REQUEST).send();
}
const controllerRequest = req.body;

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,

Check warning

Code scanning / CodeQL

Exception text reinterpreted as HTML Medium

Exception text
is reinterpreted as HTML without escaping meta-characters.
headers: controllerRequest.headers || null,
Expand Down
116 changes: 100 additions & 16 deletions src/server/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import express = require('express');
import * as fs from 'fs';
import * as path from 'path';
import * as dns from 'dns';
var escape = require('escape-html');
import { SERVER_ERROR, SUCCESS } from './serverBase';

export const fetchDrivesOnWindows = (res: express.Response) => {
Expand All @@ -15,27 +17,74 @@ export const fetchDrivesOnWindows = (res: express.Response) => {
});
};

const SAFE_ROOTS_WINDOWS = [
"C:\\Users", // User home directories
"D:\\", "E:\\", // Additional drives (non-system)
"C:\\ProgramData", // Application-wide data storage
"C:\\inetpub", // IIS web server root
"C:\\Temp" // Temporary storage (watch for security concerns)
];

const SAFE_ROOTS_LINUX = [
"/home", // User home directories
"/Users", // macOS user home directories
"/var/www", // Web server content
"/var/data", // Common data storage
"/srv", // Server files
"/opt", // Optional software installs
"/workspace", // Dev environments
"/app", // Application root
"/tmp", "/var/tmp" // Temporary storage (watch for security concerns)
];

const isPathSafe = (dir: string): string | null => {
const resolvedPath = path.resolve(fs.realpathSync(dir));

// Use platform-specific safe roots
const safeRoots = process.platform === "win32" ? SAFE_ROOTS_WINDOWS : SAFE_ROOTS_LINUX;

for (const root of safeRoots) {
const resolvedRoot = path.resolve(root);

if (resolvedPath.startsWith(resolvedRoot + path.sep)) {
// Normalize the path within the safe root
return path.resolve(resolvedRoot, path.relative(resolvedRoot, resolvedPath));
}
}

return null; // Not in a safe root
};


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 {
const safePath = isPathSafe(dir);

if (!safePath) {
return res.status(403).send({ error: `Access denied: Unsafe directory ${dir}.` });
}

const result: string[] = [];
for (const item of fs.readdirSync(safePath)) {
try {
const itemPath = fs.realpathSync(path.join(safePath, item));

// Ensure itemPath is still within safePath
if (itemPath.startsWith(safePath + path.sep)) {
const stat = fs.statSync(itemPath);
if (stat.isDirectory()) {
result.push(item);
}
}
} catch {
// Ignore errors 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 +129,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 7926c99

Please sign in to comment.