Skip to content

fix(ui): server-side request forgery api x-endpoint #2920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix(ui): server-side request forgery api x-endpoint #2920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pages/api/media-type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,26 @@ export default async function mediaTypeHandler(req: NextApiRequest, res: NextApi
try {
const url = getQueryParamString(req.query.url);

// SSRF protection: Only allow URLs from trusted hostnames
const allowedHostnames = [
'example.com',
'cdn.example.com',
// Add other trusted hostnames as needed
];
let parsedUrl;
try {
parsedUrl = new URL(url);
} catch (e) {
httpLogger.logger.error({ message: 'Invalid URL', url });
res.status(400).json({ type: undefined, error: 'Invalid URL' });
return;
}
if (!allowedHostnames.includes(parsedUrl.hostname)) {
httpLogger.logger.error({ message: 'Disallowed hostname', url, hostname: parsedUrl.hostname });
res.status(403).json({ type: undefined, error: 'Disallowed hostname' });
return;
}

Comment on lines +14 to +33
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main characteristic of this resource is that we do not know the domain of the server that serves the asset in advance. Therefore, we cannot determine the allowed hostname array at all.

const end = metrics?.apiRequestDuration.startTimer();
const response = await nodeFetch(url, { method: 'HEAD' });
const duration = end?.({ route: '/media-type', code: response.status });
Expand Down
19 changes: 18 additions & 1 deletion pages/api/proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,32 @@ import fetchFactory from 'nextjs/utils/fetchProxy';

import appConfig from 'configs/app';

// Define an allow-list of permitted endpoints
const ALLOWED_ENDPOINTS = [
appConfig.apis.general.endpoint,
// Add other allowed endpoints here, e.g.:
// "https://api.example.com",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This resource is not used in production, so the changes are not necessary. If we want to keep them, please add all available API endpoints to the list.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I don't see how this resource can be requested using the endpoint from user input.

];

const handler = async(nextReq: NextApiRequest, nextRes: NextApiResponse) => {
if (!nextReq.url) {
nextRes.status(500).json({ error: 'no url provided' });
return;
}

// Validate x-endpoint header against allow-list
let baseEndpoint = appConfig.apis.general.endpoint;
const requestedEndpoint = nextReq.headers['x-endpoint']?.toString();
if (requestedEndpoint && ALLOWED_ENDPOINTS.includes(requestedEndpoint)) {
baseEndpoint = requestedEndpoint;
} else if (requestedEndpoint) {
nextRes.status(400).json({ error: 'Invalid endpoint' });
return;
}

const url = new URL(
nextReq.url.replace(/^\/node-api\/proxy/, ''),
nextReq.headers['x-endpoint']?.toString() || appConfig.apis.general.endpoint,
baseEndpoint,
);
const apiRes = await fetchFactory(nextReq)(
url.toString(),
Expand Down
6 changes: 6 additions & 0 deletions ui/address/details/AddressSaveOnGas.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,18 @@ const AddressSaveOnGas = ({ gasUsed, address }: Props) => {

const gasUsedNumber = Number(gasUsed);

// Simple Ethereum address validation (42 chars, starts with 0x, hex)
const isValidAddress = /^0x[a-fA-F0-9]{40}$/.test(address);

Comment on lines +28 to +30
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that if the address is invalid, the page will throw a 422 error, so this code will never execute.

const query = useQuery({
queryKey: [ 'gas_hawk_saving_potential', { address } ],
queryFn: async() => {
if (!feature.isEnabled) {
return;
}
if (!isValidAddress) {
throw new Error('Invalid address format');
}

const response = await fetch(feature.apiUrlTemplate.replace('<address>', address));
const data = await response.json();
Expand Down