The proposal did let the BA specify the path to the executable, but it had
to reference the Id of the ApprovedForElevationExe which meant that the
engine could look up its hash and verify that the executable at the path
specified was indeed the one the BA was asking for. Except you're kind of
raining on my parade saying that it won't be able to verify it at arbitrary
paths.
On Fri, May 16, 2014 at 7:17 PM, Rob Mensching <r...@firegiant.com> wrote:
> Trust me, it isn’t possible. I tried it that way first and had to
> retroactively add the whole “.unverified” stuff due to a huge security
> vulnerability that was possible without it. Code was much more simple
> before I had to do that. <sigh/>
>
>
>
> I thought the proposal included a “path\to\launch.exe”. That’s why I
> brought up the use case. Seems like the path should be unnecessary. Just
> provide an identifier and then maybe let the BA pass command-line arguments.
>
>
>
> _______________________________________________________________
>
> FireGiant | Dedicated support for the WiX toolset |
> http://www.firegiant.com/
>
>
>
> *From:* Sean Hall [mailto:r.sean.h...@gmail.com]
> *Sent:* Friday, May 16, 2014 5:12 PM
> *To:* WiX toolset developer mailing list
> *Subject:* Re: [WiX-devs] WIP3249 - Allow BA to Run Elevated Async
> Process Through the Engine
>
>
>
> I didn't check whether you really could lock it in such a way that no one
> can modify it until the Windows loader loads it, I just assumed that was
> possible. If that's not possible, then that means we would have to require
> it's in a secure location like you mentioned. I thought the ".unverified"
> dance was because it saves it for later, not because that was the only way.
>
>
>
> The proposal isn't for any executable, it's for any executable that you
> specified at compile time (which the binder would hash and stuff). I'm not
> sure if you missed that, or didn't consider it because of the verification
> problem.
>
>
>
> Right, I always install my config tools.
>
>
>
> That's why I don't get sick :) I'm glad you're feeling better.
>
> On Fri, May 16, 2014 at 6:38 PM, Rob Mensching <r...@firegiant.com> wrote:
>
> To be clear, the real security concern I have is the ability for the BA
> to launch any executable elevated. The real security threat is if the
> engine provides an API that says
> IBootstrapperEngine::LaunchConfigToolElevated(LPWSTR sczFullPathToTool).
> Then a **non-elevated** process can insert a thread into the non-elevated
> BA and call that engine API to launch any executable elevated. That’s much
> badness.
>
>
>
> Note: verifying a file is the correct file on disk won’t help if the file
> is not placed in a secure location. The issue is that while you can lock
> the file to verify it, you either have to unlock the file to let the
> Windows loader load the file or open the file “share for delete” (because
> the Windows loader will load the file with “share for delete” as well). If
> you do “share for delete” then another process can move the file you are
> verifying out of the way and put the bad executable it its place. This is
> why Burn does the whole “.unverified” dance when caching elevated payloads,
> to ensure that files cannot be tampered with between verification and
> placement in their final resting location.
>
>
>
> The proposal to include an executable path (that probably has to be
> configured with variables) in the Burn manifest is probably the necessary
> way to go. I suppose we could allow several different well known
> identifiers that each point at a path to be launched.
>
>
>
> With this feature, the user does have to be conscious of the security
> implications. They too must point to files that were installed to safe
> locations (ProgramFilesFolder is safe). In all likelihood they already have
> a variable that points to the root of that path (InstallFolder) and
> hopefully can just tack on to that, like
> “[InstallFolder]my\path\to\configurationtool.exe”.
>
>
>
> I also agree that I don’t like the design of using a “fake ExePackage” for
> this scenario. That seems like overloading the “package” concept to be
> something that isn’t a package.
>
>
>
> I also don’t know how much we should bother adding executables to the
> Bundle that aren’t just installed files. If the config tool is at all
> interesting, it probably has dependencies and stuff that is shared with the
> larger application. So, IMHO, it seems better to just tell people to
> install their config tool and allow the Bundle+BA to launch it at the right
> time.
>
>
>
> PS: Sorry I’ve been absent on these interesting design discussions. Being
> sick sucked.
>
>
>
> _______________________________________________________________
>
> FireGiant | Dedicated support for the WiX toolset |
> http://www.firegiant.com/
>
>
>
>
> ------------------------------------------------------------------------------
> "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
> Instantly run your Selenium tests across 300+ browser/OS combos.
> Get unparalleled scalability from the best Selenium testing platform
> available
> Simple to use. Nothing to install. Get started now for free."
> http://p.sf.net/sfu/SauceLabs
> _______________________________________________
> WiX-devs mailing list
> WiX-devs@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/wix-devs
>
>
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
WiX-devs mailing list
WiX-devs@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/wix-devs
