grke / burp

burp - backup and restore program
http://burp.grke.net
Other
481 stars 77 forks source link

win64 client: Password and priv keys accessible by local Users group #826

Open BlessDeix92 opened 5 years ago

BlessDeix92 commented 5 years ago

I am testing burp for the first time and saw that after installing the client on a Windows 10 system that the entire burp install directory and all files are readable by the local Users group, which means unprivileged local users have read access to things like the client password and private TLS keys.

This seems kinda bad.

I am testing with version 2.2.18, which is the current stable release.

grke commented 5 years ago

Hello,

Thanks for your interest.

What is your suggestion for fixing this? The code for the installer is in src/win32/installer if you would like to take a look.

BlessDeix92 commented 5 years ago

Recursively removing Users access from the burp install directory should do it. Pretty easy in theory. I don't think changing that would negatively affect any legitimate usage.

I'll check out the installer source and see if I can help. It's been awhile but I've done NSIS installers before.

grke commented 5 years ago

Awesome, thanks for your input.

tallship commented 5 years ago

Wow. That's kinda way more than kinda bad. I've never used BURP or Bacula for Windows DR anyway, so I wouldn't have noticed this, but I do have an RFP sitting in my inbox right now for such a deployment, so I'll be watching this closely for resolution.

In the meantime, a kludgy work-around will suffice if my proposal is (as expected) accepted by the customer.

deajan commented 5 years ago

@BlessDeix92 I don't think NSIS provides an easy way to remove those ACL out of the box (but there are plugins, see https://nsis.sourceforge.io/Talk:AccessControl_plug-in) You might have an easier time using NSIS to call icacls at the end of the install process like:

ExecWait '$SYSDIR\icacls.exe /T /inheritance:r /grant:r S-1-5-18 $INSTDIR\*'
ExecWait '$SYSDIR\icacls.exe /T /inheritance:r /grant S-1-5-32-544 $INSTDIR\*'
BlessDeix92 commented 5 years ago

A better idea might be to move configuration and generated files out of Program Files completely and put them in somewhere like ProgramData instead.

Hey grke, what's the minimum Windows version you want to support?

I'll look into this over the weekend. I don't even know if burp uses NSIS or if it's a different installer! I'll figure it out later.

For now people can just recursively remove Users permissions from the burp install directory and that fixes it.

For most small business and home environments where there is only a single local user who has local admin rights anyway, this issue has no negative impact. That's probably >90% of the userbase.

grke commented 5 years ago

Hello,

Burp uses NSIS. Fairly recently, I containerised the Windows build tools, here: https://github.com/grke/burp-cross-tools Previously, it was very difficult to get Windows builds working, but this makes it quite easy - though you still need to source the vss headers yourself.

I don't know much about Windows versions, except that burp worked under XP at some point in the past and I don't know whether it still does. I have test Windows 2008 server VMs, so let's say Windows 2008.

grke commented 5 years ago

Re: "kinda way more than kinda bad" Here is what I have always stated about burp, displayed prominently on the burp website:

It is open source free software (where 'free' means both that you do not have to pay for it, and that you have freedom to do what you want with it) released under the AGPLv3 licence. See the LICENCE for more information.

Finally, as with the vast majority of open software, Burp comes with absolutely no warranty. You are responsible for testing it and ensuring that it works for you.

Having said that, it is really great to see people finding ways that they would like to improve burp, thanks for all the input.

tallship commented 5 years ago

lolz, yes. That was me :)

Like I said, I've never had much use for using BURP or Bacula as a DR solution for Windows. In fact, starting at the career point in my life of 1995, As an MCT for Microsoft I had two solidly firm objectives:

1.) To teach my students to pass their exams, earning their MCSE certifications

2.) To poach new customers so I could convince them to eradicate Microsoft products from their enterprise and use Linux and the BSDs instead.

I got greenlighted on that contract, which plays out like this:

Build an onsite private cloud based on SuSE hosts with KVM virtual machines for Windows workstations.

Blow away Windows on the desktop, install Slackware Linux and manage those installations with Puppet, using Remmina to access their virtual Windows machines as workstations when need be.

Aside from snapshots, Duplicity, and plain old tar & rsync for critical, smaller parts of the file systems, BURP figures into the larger DR plan as a whole.

Perhaps I should have also mentioned that it's, "kinda way more than kinda bad to use Microsoft Windows", in the first place lolz.

No complaints here, BURP is the foshizzle! :)

hakong commented 3 years ago

This should probably have been reported privately as a security vulnerability (potential privilege escalation, arbitrary file read) and a 90 day window given to fix this without going public. Even assigned a CVE :) Effectively, this means every user on the client has full access to the whole filesystem.

As a standard user, I can now access any file on the computer by restoring it from the burp server. Steps to reproduce:

With a complete copy of the target's file system it shouldn't be hard to find some credentials and work your way towards administrative access to the target.

The solution suggested by @BlessDeix92 could be a good starting point. Move all custom data (config, certificate data, logs) out of Program Files and somewhere where standard users have no access to.

grke commented 3 years ago

Hello,

Thank you for reminding me about this. I have made a change in git master, for the next release (2.5.0). It sets the ownership of the ssl files to SYSTEM and Administrator on initial certificate exchange. It also sets the ownership of the ssl files and the burp.conf to the same users after the installer runs. I don't know if this will work for all versions of Windows, but it's a start.