Skip to content
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.

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

fix: allow any origin for selfhost #1397

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Conversation

julianpoy
Copy link
Owner

No description provided.

@julianpoy julianpoy enabled auto-merge June 6, 2024 23:28
@julianpoy julianpoy merged commit 62b21ad into master Jun 6, 2024
2 of 3 checks passed
@julianpoy julianpoy deleted the allow-any-origin-selfhost branch June 6, 2024 23:30
@Sobuno
Copy link

Sobuno commented Jun 7, 2024

Could it be possible to define this list ourselves for self-hosting instead? I am not really comfortable with having CORS set to any origin

@julianpoy
Copy link
Owner Author

@Sobuno can you clarify what makes you uncomfortable? Are you concerned someone may embed your selfhosted version in their site?

@Sobuno
Copy link

Sobuno commented Jun 8, 2024

By allowing CORS from any site, visiting a site on the Internet can allow that site to start sending XHR requests to RecipeSage.

Since you do not set Access-Control-Allow-Credentials (and since you're not using cookies for session storage anyway), I am not that worried about authenticated endpoints.

What I am worried about is network enumeration. By allowing any site on the Internet to attempt to hit endpoints, attackers can discover that I have RecipeSage running at IP X, Port Y.

Here's an example of a script a malicious website could host:

async function discoverRecipeSage(subnet) {
	let promises = [];
	for(var i = 0; i<255; i++) {
			promises.push(checkForRecipeSage(`http://${subnet}.${i}:7270/api/trpc/getRecipes`));
	}
	
	let results = await Promise.all(promises);
	let recipeSageInstances = results.filter(value => value !== null);
	return recipeSageInstances;
	  
}


async function checkForRecipeSage(address) {
	try {
		let response = await fetch(address)
		let body = await response.json();
		//Using some kind of marker to detect that this is RecipeSage - Could be refined to something more clever than this check, possibly obtaining the RecipeSage version at the same time
		if(body.error.json.code == -32600) {
			console.log("Found RecipeSage at " + address);
			return address;
		}
		return null;
	} catch(exception) {
		//CORS blocks us, we have no idea if RecipeSage runs here
		console.log(exception);
		return null;
	}
}

let instances = await discoverRecipeSage("<some subnet prefix here, e.g. 192.168.1>")
//TODO: Ship this information to my own server for mapping out my target's network

The potential attacker now knows I have RecipeSage running in my internal network at a specific IP. If at some point an exploit is discovered in some versions of RecipeSage (E.g. remote code execution or some escalation of privileges issue), the attacker might be able to use this information to perform an actual attack rather than just the initial information gathering.

When CORS is not allowed for a specific site, fetch (or any other XHR-based request, i.e. the old and trusty XmlHttpRequest) will simply throw an error and the response will not be passed on to the Javascript code in the malicious site (It will still be visible in the Network tab of Chrome or Firefox which can confuse some, but the malicious site cannot read this information)

@Sobuno
Copy link

Sobuno commented Jun 8, 2024

The above script will result in a ton of network errors in the Network tab of the browser, but normally you don't have that open while browsing random websites.

Testing it out locally, enumerating all 255 IPs takes 24 seconds - although depending on which order the Promises are executed by the Javascript engine, the actual instance may be discovered much more quickly

(Of course to target the particular subnet, you'd have to know the user's local IP - which was possible through WebRTC until a few years ago)

Not using the default port and default subnets of course reduces the issue here, but I'd prefer not to resort to security through obscurity :)

@julianpoy
Copy link
Owner Author

Reasonable enough! I'll add an optional environment variable to specify a comma separated list of origins

@julianpoy
Copy link
Owner Author

#1398

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants