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

What happens if a different user connects to my running private comments server? #23

Open
filmil opened this issue Sep 21, 2023 · 8 comments
Labels
question Further information is requested

Comments

@filmil
Copy link
Contributor

filmil commented Sep 21, 2023

Is such a scenario possible?

@masukomi
Copy link
Owner

1st you'd have to have to manually expose port 5749 to the network which. Normally another computer wouldn't be able to see that port on yours, so by default no it's not possible. The intention is to run this locally and not share server access with others. There's not a good reason too (see below)

However, if you expose it, what happens depends on what the other person knows.

1st the non-nefarious case.
A coworker clones down the same repo "foo" AND they're running the private comments plugin in their editor AND they've configured it to look at your server instead of their local computer.

They'd see the same info you saw. They'd be able to add, change, and delete comments.

This is actually intentional, although it's not intended that you'd do it through the same server.

This came from the use case of working at an agency hired to come in and fix an important codebase that had been abandoned. The code was crap, and I ended up leaving a LOT of breadcrumbs for myself, because we couldn't just refactor everything we saw. We had to be pretty focused on high priority fixes first. So, you'd leave notes like "this method is dependent upon Foo being configured. See " While you wouldn't want to write anything inappropriate those breadcrumbs were invaluable as we learned how the new system worked. THere was just too much to keep in your head.

With PC you could share the same git repo. Just make a cron job that syncs your private comments to a private repo on a shared server like github. Your teammates do the same and now you're collaboratively leaving comments about the code but not IN the code.

2nd the bad actor

The hashing is of project and file names is consistent. For example (from one of the comments in the code)

    ( (project_name_hash . 1aabeb680a9ed12e4fb53d529513a2aa58341f5cfd0f7790c96388cbddbd5493)
       (file_path_hash . 00c5f3f22a7c1dc3b2e377b650276b6027642ba5b21dc3bb132997f39065870a)
       (treeish . #f)
       (line_number . 4)
     )

let's say that your repo is cloned into a folder named "foo" AND let's say that that hashes to "1aabeb680a9ed12e4fb53d529513a2aa58341f5cfd0f7790c96388cbddbd5493"

IF your attacker knew that you were working in a repo named foo they could (after looking at the source code of PC) figure out the project_name_hash. If they ALSO knew the path to one of the files within that repo they could work out the file_path_hash. With those two pieces of information they could iteratively extract a copy of every comment you left on that file, and your changes to them.

They'd have to make a series of requests like this.

GET http://0.0.0.0:5749/v1/comments?project_name_hash=1aabeb680a9ed12e4fb53d529513a2aa58341f5cfd0f7790c96388cbddbd5493&file_path_hash=00c5f3f22a7c1dc3b2e377b650276b6027642ba5b21dc3bb132997f39065870a&treeishes

BUT practically speaking they're not going to have access to your server because you're not going to open that port , and if they've already got the repo name and the structure of all the files, they've got the repo which is probably WAY more useful and valuable than whatever random comments you may have left on it. Are they really going to want to go through the time and effort to figure out what comments you've left?

So, yes, IF you open the port AND you have a boss or coworker who is a developer who really has it out for you AND you've left nasty comments on code they could, in theory, get them.

tl;dr: don't open the port. If you have coworkers who'd do 💩 like that to you be careful what you write EVERYWHERE, not just in PC.

Speaking for myself the worst anyone would find is comments saying "this method is really frustrating. You need to do X to make ti work. Bob's the person to go to with questions."


side note: if for some reason you're working on a shared server and DO intend to share a PC server it would work but you can encounter race conditions. For example you load a comment, a coworker deletes it, and then you try and edit it. In practice the likelyhood of two people working on the same comments in the same file at essentially the same time are pretty low.

@filmil
Copy link
Contributor Author

filmil commented Oct 26, 2023

I would instead recommend requiring the use of FIFOs in the filesystem, since access to those is subject to file permissions.

You don't need malicious coworkers to get bitten here.
It's enough for a piece of malicious code to connect to the port and perhaps exploit a buffer overflow (or some such) to get at much more than just your code comments.

And having a long term open port is an issue anyways: where I work, your machine can get reset, rebooted, or kicked off the corporate network if it's keeping random open ports.

@masukomi masukomi added the question Further information is requested label Oct 26, 2023
@masukomi
Copy link
Owner

I feel like I'm missing something important here. This isn't a port that's open by default on most (any?) systems. If you don't want malicious actors to connect to the port, to get data, or exploit buffer overflow, don't expose it.

How is that not sufficient data privacy? The files remain just a safe as anything on your computer, and there's nothing exposed that a malicious actor could exploit.

There is zero need to expose any ports to the network for Private Comments to function as designed.

If you do want to share the comments with team-mates, that would best be done via git (use cron or something to push changes to repo on regular basis, and pull regularly) which you're presumably already doing for the repo you're commenting on.

@filmil
Copy link
Contributor Author

filmil commented Nov 17, 2023

How is that not sufficient data privacy?

It simply isn't, but I can't go into details.

For this reason, my employer forbids random open ports on machines.
As a result I can not use private_comments where I'd like to.

@masukomi
Copy link
Owner

For this reason, my employer forbids random open ports on machines.

But, PC doesn't need an open port. Like, not in the normal sense of "this port is open to the network".

I feel like "thou shalt not install any app that uses ports even if not exposed to the network" is like... ⁉️🤯 like that just breaks so many inter-app things and i (possibly ignorantly) don't see how that makes anything safer. Also confused by the suggestion to switch it to an LSP in this context because LSPs use ports too...

@filmil
Copy link
Contributor Author

filmil commented Nov 17, 2023

But, PC doesn't need an open port. Like, not in the normal sense of "this port is open to the network".
[...] don't see how that makes anything safer.

I don't know if may go into details or not, so I won't.

tl;dr: please update your security posture to modern standards.

Also confused by the suggestion to switch it to an LSP in this context because LSPs use ports too...

I think LSP uses stdin/stdout. Any two-way transport would do, but I suspect not using TCP/IP was deliberate.

Also you don't need to switch anything really, a LSP server shim is all that is required.

@filmil
Copy link
Contributor Author

filmil commented Nov 17, 2023

And btw, one easy change that could be made is to use a FIFO instead of a hostport.
You can then place the FIFO somewhere in a filesystem you only can access.
No need to fiddle with firewalls, especially if you aren't the one who controls the firewall policy on your machine.

@filmil
Copy link
Contributor Author

filmil commented Jan 17, 2024

tl;dr: please update your security posture to modern standards.

Happy new year! I apologize for the uncalled for rudeness in this comment. I thought I was being factual, but reflecting on it, I was actually being needlessly combative. I promise to do better in the future.

Bottom line is that tcp, unix sockets and std{in,out} have their place in different settings. The decision, of course, is yours.

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

No branches or pull requests

2 participants