Using Encrypted Cookies instead of Local Storage, IPC Sender Verification, and Preventing Unintended Reloads
- Dominant language
- TypeScript
- Stars
- 40k
- Forks
- 2.4k
- Avg merge
- 2d 13h
- Merged PRs (30d)
- 61
Description
### Expected Behavior
1. The app can store sensitive information in encrypted cookies on the user's filesystem.
2. The app can verify all IPC message senders before responding to them.
3. The app can prevent reload of a window when handling navigation to the `appUrl`.
### Actual Behavior
1. The app uses local storage in an unencrypted manner.
2. The app does not verify `event.sender` on all IPC messages.
3. The app `return`s from `will-navigate` if the navigation is to `appUrl` but does not `preventDefault()`.
### Reproduction Steps
Thank you for designing the Insomnia Desktop Application and for making its source code available. The app does a great job of handling in-app navigation and following secure practices. We list a few pointers that can help make the app more secure.
1. [Encrypting Cookies] The app currently stores sensitive information, including secret key, etc. in local storage. It will be beneficial to use cookies for the same and to encrypt them on the file system using Electron’s fuses. [[Link](https://www.electronjs.org/docs/latest/tutorial/fuses#cookieencryption)]
2. [IPC Verification] Since the app currently uses custom IPC (e.g., for Socket Handlers [[Ref](https://github.com/Kong/insomnia/blob/a5d843171a841b9117e2e4b1503a362fca6bcfd0/packages/insomnia/src/main/network/websocket.ts#L497)]), it may be helpful to integrate sender verification `event.sender` throughout before handling IPC messages in the main process. [[Link](https://www.electronjs.org/docs/latest/tutorial/security#17-validate-the-sender-of-all-ipc-messages)]
3. [Preventing Reloads] The current event handler on ‘will-navigate’ simply returns if the URL matches the `appUrl` [[Ref](https://github.com/Kong/insomnia/blob/a5d843171a841b9117e2e4b1503a362fca6bcfd0/packages/insomnia/src/main/window-utils.ts#L106)]. It will be helpful to consider adding an `event.preventDefault()` here as well unless a reload is intended for the window.
### Is there an existing issue for this?
- [X] I have searched the [issue tracker](https://www.github.com/Kong/insomnia/issues) for this problem.
### Additional Information
–
Mir Masood Ali, PhD student, University of Illinois Chicago
Mohammad Ghasemisharif, PhD Candidate, University of Illinois Chicago
Chris Kanich, Associate Professor, University of Illinois Chicago
Jason Polakis, Associate Professor, University of Illinois Chicago
### Insomnia Version
8.4.5
### What operating system are you using?
macOS
### Operating System Version
12.7.1
### Installation method
download from insomnia.rest
### Last Known Working Insomnia version
_No response_
Contributor guide
Research direction
Start with packages/insomnia/src/main/network/websocket.ts and packages/insomnia/src/main/window-utils.ts, then trace the local-storage usage and other main-process IPC handlers. Compare the relevant Electron security guidance with the current behavior; done means sensitive storage is encrypted, IPC senders are validated, and matching appUrl navigation cannot unintentionally reload the window.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- electron, typescript
- Domain
- desktop, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100