New issue
Advanced search Search tips

Issue 888 link

Starred by 1 user

Issue metadata

Status: Fixed
Closed: Nov 2016

Sign in to add a comment

1Password: Process Authentication Breaks Local Security

Project Member Reported by, Aug 8 2016

Issue description

There are a number of problems with the security model of 1Password that results in the local security model being disabled, as well as a number of security, sandboxing and virtualization features.

1Password works by creating a WebSocket server on localhost, and then the browser extension connects to ws:// and requests passwords without any authentication other than the Origin header. Naturally, users can lie about the Origin, so when a client connects to the WebSocket port the server uses GetExtendedTcpTable() and then searches for a matching client. This is similar to running netstat to see who owns a socket.

When a match is found, 1Password attempts to OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_DUP_HANDLE | PROCESS_VM_READ) the process id to verify that it's owned by the right user. If that succeeds, they query the image filename and use WinVerifyTrust() and ImageEnumerateCertificates() to check if the image was signed by a known browser vendor.

There are a number of problems with this approach.

Firstly, OpenProcess() succeeding does not imply that the process is trusted. Any unprivileged user can attach a NULL or empty DACL to a process, something like this:

    InitializeSecurityDescriptor(&Descriptor, SECURITY_DESCRIPTOR_REVISION);
    SetSecurityDescriptorDacl(&Descriptor, TRUE, NULL, FALSE);

    Attributes.nLength              = sizeof(SECURITY_ATTRIBUTES);
    Attributes.bInheritHandle       = TRUE;
    Attributes.lpSecurityDescriptor = &Descriptor;
    StartupInfo.cb                  = sizeof(STARTUPINFO);


That will permit anyone else to OpenProcess() it. The trust verification on imagefiles is inherently vulnerable to TOCTOU, and therefore any user can dump the passwords of any other user. I've attached two examples, allowopen.c that will create a process with an empty DACL and testopen.c that does the same authentication that 1Password does for testing.

This bug is subject to a 90 day disclosure deadline. If 90 days elapse
without a broadly available patch, then the bug report will automatically
become visible to the public.

1.2 KB View Download
402 bytes View Download
Project Member

Comment 1 by, Aug 8 2016

Hah, I just found out why you need PROCESS_VM_READ to get the Image Filename - it's literally just reading it out of the PEB (Thanks to James for the hint).

This means you don't even need to bother with the TOCTOU, just set NtCurrentTeb()->ProcessEnvironmentBlock->ProcessParameters->ImagePathName.Buffer to whatever filename you want, and 1Password will verify the trust of the wrong file.

Project Member

Comment 2 by, Aug 8 2016

Response from 1Password:

Acknowledged. And I found it great to meet you in Vegas.
Thank you for taking the time to do so.

And thank you for your report, explanation and PoC. Those
will make it easier for us to get this fixed. We should be able to
test and confirm this soonish.

Although we still do need to test, what you say and report is fully
consistent with how 1Password works, and so I suspect that we
will be able to confirm vulnerability.

Although it is too early to say, a counter measures we are
exploring include using an authenticated connection between browser
client and Agent. This would require a long term secret stored stored
by the browser. This would not defend against an attacker running as
the target user, but it would defend against the attacker in your case
where the the attacker is a separate unprivileged user running on the same
host. This would then be a Trust on First Use situation.

Additionally, we will explore testing for an empty/NULL DACL, to rule
out the very specific attack, though mutual authentication between
the web socket server and the 1Password browser extension would provide
a more robust defense against a broad family of attacks.

Again thank you.

You should here from us well before 90 days!

Project Member

Comment 3 by, Nov 12 2016

Labels: -Restrict-View-Commit -Severity-High Severity-HIgh
Status: Fixed (was: New)

Comment 4 by, Nov 13 2016

I am Jeffrey Goldberg of AgileBits, the makers of 1Password.

Thank you Tavis and everyone from Project Zero for your research and help in making 1Password safer.

This was fixed in 1Password for Windows released on 2016-10-06. The release notes start out with:

> Improves verification and authentication of communication with the 1Password browser extension to address an issue reported by Tavis Ormandy of Google Project Zero. Users should update promptly.

We have further described the issue and the approach we took to fix this in

Sign in to add a comment