sandboxie-plus / Sandboxie

Sandboxie Plus & Classic
https://Sandboxie-Plus.com
GNU General Public License v3.0
13.69k stars 1.52k forks source link

Sandboxie-Plus > Snapshot Manager UI: Usability issues [priority:low] #350

Closed typpos closed 3 years ago

typpos commented 3 years ago

Looking at Sandboxie-Plus v0.5.4b, Snapshot Manager:

1. Documentation

Is there any documentation? I could not find anything useful on sandboxie-plus.com, nor in repo search for "snapshot".

2. Security issue re access rights to snapshots.ini

3. Unable to snapshot empty sandbox

If sandbox is empty, the Sandbox Manager allows user to select "Take Snapshot" but then returns a somewhat cryptic error message, "Failed to create directory for new snapshot".

4. Snapshot details vs "Take Snapshot" confusion

Having "Take Snapshot" sit next to "Name" text box may be confusing. I incorrectly entered the name of a new snapshot I wanted to create in the "Name" field and then clicked "Take Snapshot" to "create a snapshot by that name and description". This unexpectedly changed the snapshot name of the old snapshot.

5. "Go to snapshot" and "Remove snapshot" are enabled when no snapshot exists

Button "Go to snapshot" is enabled when no snapshot exists. Application shows "are you sure" and then "Snapshot not found".

6. Empty snapshot name is not blocked

7. Duplicate snapshot name is not blocked

8. Tree control header colour

Using "Bright UI Theme", the "Snapshot" header text is just text without background highlight, making it look like a clickable item. In "Dark UI Theme" it's ok, so long as there is at least one snapshot below it. For a new user, the display of the heading "snapshot" (dark or bright theme) can be confusing when there are no snapshots yet as it is not obvious that this is the header row of an empty tree control.

9. Double click on parent snapshot triggers close/expand tree

Double-clicking a snapshot in the tree first shows a dialog to ask if snapshot should be activated. If answered no, the tree node unexpectedly toggles collapsed-state because the mouse event is not canceled/marked consumed.

10. Aborted snapshot creation can make future snapshots impossible/problematic

If something goes wrong during snapshot creation, the new snapshot-xx folder is partially created and then the application aborts with an error message. Thereafter, snapshot creation may not work or keep showing errors. Different scenarios apply.

Example: Open (non sandboxed) command prompt to browse sandboxie folders. Now create a snapshot which would try to move the currently locked folder. Error message shows. Close the command prompt. Create another snapshot. Error shows.

11. Suggestion: Snapshot Manager as tab in main window

The Snapshot Manager window might work great as a new panel in the tab control in the main window, next to "Sbie Messages" and "Resource Monitor".

DavidXanatos commented 3 years ago
  1. volunteers welcome
  2. its not a big issue as it does not allow for a sandbox escape, worst case is a malicious application destroys the sandbox content
  3. fixed in next build, added propper error message
  4. fixed in next build
  5. fixed in next build
  6. is that an issue though the name is only for display and if you forgot to set one you can do that later? unlike trying to creating a sandbox with no name there is nothign this breaks
  7. see 6
  8. how to fix this?
  9. fixed in next build
  10. I know I need to fix that...
  11. may be
typpos commented 3 years ago

1 I'll beautify it if you give me dot-points. I have no clue what this is meant to do and how registry/files are treated.

2 Fine and very low priority. It's just yet another one of those "need to think to be sure" security risk things that will also need to be kept in mind in future work. For example, one day someone adds features to snapshots that suddenly do create a problem, like snapshot-based permissions that toggle with changing snapshots etc.

6 & 7.: (low priority) Well, technically "works", but from a usability and user-expectation perspective? Look at the tree of snaps when there are one or more without names: Looks rather broken. Likewise, if you disallow "empty" in one place, why would you allow it in another?

8 Options: (a) Improve Qt colour scheme; (b) float a label, xy-centered, over the tree control saying "No snapshots yet!"; (c) make the root-label the sandbox itself (I don't understand snapshots so this may not make sense)

10 I should post a screenshot of your comment to reddit r/ProgrammerHumor :-)

DavidXanatos commented 3 years ago

what are dot points? the registry is always fully copied for each snapshot, files are merged from all parent snapshot folders together.

NewKidOnTheBlock commented 3 years ago

re 1) We could use the Wiki section.

µblock origin serves as an awesome example: https://github.com/gorhill/uBlock/wiki I've learned a lot by reading through that Wiki.

typpos commented 3 years ago

re 1) We could use the Wiki section.

You don't like my fancy work!? :-)

DavidXanatos commented 3 years ago

did i miss anything? i think I can close this one

norelmas commented 3 years ago
  • volunteers welcome
  • its not a big issue as it does not allow for a sandbox escape, worst case is a malicious application destroys the sandbox content
  • fixed in next build, added propper error message
  • fixed in next build
  • fixed in next build
  • is that an issue though the name is only for display and if you forgot to set one you can do that later? unlike trying to creating a sandbox with no name there is nothign this breaks
  • see 6
  • how to fix this?
  • fixed in next build
  • I know I need to fix that...
  • may be

I took snapshots and selected a snapshot, after I confirm to go to that snapshot, sometimes it says "Snapshot not found". It's a different issue that happens when a sandbox has snapshots.