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 7926c99 commit 153fa17
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 60 deletions.
11 changes: 1 addition & 10 deletions src/app/api/services/localRepoService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,5 @@ export const fetchLocalFileNaive = async (path: string, fileName: string): Promi

export const fetchDirectories = async (path: string): Promise<string[]> => {
const response = await fetch(`${CONTROLLER_API_ENDPOINT}${GET_DIRECTORIES}/${encodeURIComponent(path || DEFAULT_DIRECTORY)}`);
if (!path) {
// only possible when platform is windows, expecting drives to be returned
const responseText = await response.text();
const drives = responseText.split(/\r\n/).map(drive => drive.trim()).filter(drive => drive !== '');
drives.shift(); // remove header
return drives.map(drive => `${drive}/`); // add trailing slash for drives
}
else {
return response.json();
}
return response.json();
};
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ exports[`ListItemLocalLabel matches snapshot when repo type equals Local 1`] = `
>
modelRepository.types.local.infoText
</div>
<StyledMessageBar
messageBarType={5}
>
modelRepository.types.local.warning
</StyledMessageBar>
</div>
</Fragment>
`;
Expand Down
7 changes: 5 additions & 2 deletions src/app/modelRepository/components/listItemLocalLabel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
**********************************************************/
import * as React from 'react';
import { Trans, useTranslation } from 'react-i18next';
import { Link } from '@fluentui/react';
import { Link, MessageBar, MessageBarType } from '@fluentui/react';
import { REPOSITORY_LOCATION_TYPE } from '../../constants/repositoryLocationTypes';
import { ResourceKeys } from '../../../localization/resourceKeys';

Expand All @@ -17,7 +17,10 @@ export const ListItemLocalLabel: React.FC<{repoType: REPOSITORY_LOCATION_TYPE}>
<div className="labelSection">
<div className="label">{t(ResourceKeys.modelRepository.types.local.label)}</div>
<div className="description">{t(ResourceKeys.modelRepository.types.local.infoText)}</div>
</div>}
<MessageBar messageBarType={MessageBarType.warning}>
{t(ResourceKeys.modelRepository.types.local.warning)}
</MessageBar>
</div>}
{repoType === REPOSITORY_LOCATION_TYPE.LocalDMR &&
<div className="labelSection">
<div className="label">{t(ResourceKeys.modelRepository.types.dmr.label)}</div>
Expand Down
1 change: 1 addition & 0 deletions src/localization/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,7 @@
"local": {
"label": "Local Folder",
"infoText": "Use your local folder as a model repository. The model definition files you wish to use should have a json extension. The first JSON file with an '@id' property matching the digital twin model identifier (DTMI) will be retrieved.",
"warning": "For security reasons, please select a subdirectory inside C:\\Users\\ (Windows) or /home/ (Linux/macOS).",
"textBoxLabel": "Selected folder",
"folderPicker": {
"command": {
Expand Down
1 change: 1 addition & 0 deletions src/localization/resourceKeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,7 @@ export class ResourceKeys {
infoText : "modelRepository.types.local.infoText",
label : "modelRepository.types.local.label",
textBoxLabel : "modelRepository.types.local.textBoxLabel",
warning : "modelRepository.types.local.warning",
},
mandatory : "modelRepository.types.mandatory",
notAvailable : "modelRepository.types.notAvailable",
Expand Down
10 changes: 6 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, isSafeUrl, readFileFromLocal } from './utils';
import { fetchDirectories, findMatchingFile, isSafeUrl, readFileFromLocal, SAFE_ROOT } from './utils';

export const SERVER_ERROR = 500;
export const SUCCESS = 200;
Expand Down Expand Up @@ -134,7 +134,7 @@ export const handleGetDirectoriesRequest = (req: express.Request, res: express.R
try {
const dir = req.params.path;
if (dir === '$DEFAULT') {
fetchDrivesOnWindows(res);
res.status(SUCCESS).send([SAFE_ROOT]);
}
else {
fetchDirectories(dir, res);
Expand Down Expand Up @@ -208,9 +208,11 @@ export const handleModelRepoPostRequest = async (req: express.Request, res: expr
if (!(await isSafeUrl(userUri))) {
return res.status(403).send({ error: "Forbidden: Unsafe URL." });
}


// Reconstruct a sanitized URL
const safeUrl = `${userUri.origin}${userUri.pathname}`;
try {
const response = await fetch(userUri,
const response = await fetch(safeUrl,

Check warning

Code scanning / CodeQL

Exception text reinterpreted as HTML Medium

Exception text
is reinterpreted as HTML without escaping meta-characters.
{
body: controllerRequest.body || null,
headers: controllerRequest.headers || null,
Expand Down
62 changes: 18 additions & 44 deletions src/server/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,63 +17,37 @@ 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));
}
// Dynamically determine a "Safe Root Directory"
const getSafeRoot = (): string => {
if (process.platform === "win32") {
return "C:\\Users"; // Restrict access to user directories only
}

return null; // Not in a safe root
return "/home"; // Restrict access to home directories on Linux/macOS
};

export const SAFE_ROOT = getSafeRoot();


export const fetchDirectories = (dir: string, res: express.Response) => {
try {
const safePath = isPathSafe(dir);
// Resolve the requested directory relative to the safe root
const resolvedPath = fs.realpathSync(path.resolve(SAFE_ROOT, path.relative(SAFE_ROOT, dir)));

if (!safePath) {
return res.status(403).send({ error: `Access denied: Unsafe directory ${dir}.` });
// Ensure resolvedPath is still inside SAFE_ROOT (prevents traversal attacks)
if (!resolvedPath.startsWith(SAFE_ROOT)) {
return res.status(403).send({ error: "Access denied. Unsafe directory." });
}

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

// Ensure itemPath is still within safePath
if (itemPath.startsWith(safePath + path.sep)) {
// Ensure itemPath is still inside resolvedPath (protects against symlink attacks)
if (itemPath.startsWith(resolvedPath + path.sep)) {
const stat = fs.statSync(itemPath);
if (stat.isDirectory()) {
result.push(item);
result.push(escape(item));
}
}
} catch {
Expand All @@ -83,7 +57,7 @@ export const fetchDirectories = (dir: string, res: express.Response) => {

res.status(200).send(result);
} catch {
res.status(500).send({ error: 'Failed to fetch directories.' });
res.status(500).send({ error: "Failed to fetch directories." });
}
};

Expand Down

0 comments on commit 153fa17

Please sign in to comment.