[Mingw-w64-public] [Patch] - Hardening libssp against malicious local processes
Hi,
I am putting a patch inline (not sure if attachments are allowed on this
mailing list) that does not make it possible for a local process anymore
to replace canaries which in turn might disable SSP. Comments and/or
review are much appreciated.
The problem it is trying to solve is outlined in
https://trac.torproject.org/13169#comment:4:
(Quoting a cypherpunk)
"In Windows you can to create any directories for any disks(C:, D:, ..
Z:), only system directories (Windows directory, Program files, etc) are
protected. Any process with privileges of any standard user can to
create C:\dev\urandom file and to fill it by any stuff."
And now the patch (arguably the subject can be a bit more elaborate):
From 83e3ea38a5720df52bd5f78dcd3c7b7b842ddc3b Mon Sep 17 00:00:00 2001
From: Erinn Clark
Date: Wed, 12 Mar 2014 16:09:10 +0100
Subject: [PATCH] skruffy patch
---
libssp/ssp.c | 16
1 file changed, 16 insertions(+)
diff --git a/libssp/ssp.c b/libssp/ssp.c
index aaa5a32..37f4e27 100644
--- a/libssp/ssp.c
+++ b/libssp/ssp.c
@@ -55,6 +55,7 @@ see the files COPYING3 and COPYING.RUNTIME
respectively. If not, see
/* Native win32 apps don't know about /dev/tty but can print directly
to the console using "CONOUT$" */
#if defined (_WIN32) && !defined (__CYGWIN__)
+#include
# define _PATH_TTY "CONOUT$"
#else
# define _PATH_TTY "/dev/tty"
@@ -75,6 +76,20 @@ __guard_setup (void)
if (__stack_chk_guard != 0)
return;
+#if defined (_WIN32) && !defined (__CYGWIN__)
+ HCRYPTPROV hprovider = 0;
+ if (CryptAcquireContext(&hprovider, NULL, NULL, PROV_RSA_FULL,
+ CRYPT_VERIFYCONTEXT | CRYPT_SILENT))
+{
+ if (CryptGenRandom(hprovider, sizeof (__stack_chk_guard),
+ (BYTE *)&__stack_chk_guard) && __stack_chk_guard != 0)
+{
+ CryptReleaseContext(hprovider, 0);
+ return;
+}
+ CryptReleaseContext(hprovider, 0);
+}
+#else
fd = open ("/dev/urandom", O_RDONLY);
if (fd != -1)
{
@@ -85,6 +100,7 @@ __guard_setup (void)
return;
}
+#endif
/* If a random generator can't be used, the protector switches the guard
to the "terminator canary". */
p = (unsigned char *) &__stack_chk_guard;
--
1.7.10.4
Georg
signature.asc
Description: OpenPGP digital signature
--
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [Patch] - Hardening libssp against malicious local processes
On 3. 2. 2015 11:53, Georg Koppen wrote: > I am putting a patch inline (not sure if attachments are allowed on this > mailing list) that does not make it possible for a local process anymore > to replace canaries which in turn might disable SSP. Comments and/or > review are much appreciated. > > The problem it is trying to solve is outlined in > https://trac.torproject.org/13169#comment:4: > > (Quoting a cypherpunk) > > "In Windows you can to create any directories for any disks(C:, D:, .. > Z:), only system directories (Windows directory, Program files, etc) are > protected. Any process with privileges of any standard user can to > create C:\dev\urandom file and to fill it by any stuff." Hi. This is maybe off-topic, but I'd like to point out that ACLs can be customized, so any assumption of this kind can be wrong. Also, the motivation for this patch should be "don't use /dev/urandom on Windows, because it's not a thing", not "someone could supply fake random data to us". -- David Macek smime.p7s Description: S/MIME Cryptographic Signature -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [Patch] - Hardening libssp against malicious local processes
Hi Georg,
On 02/03/15 11:53, Georg Koppen wrote:
> Hi,
>
> I am putting a patch inline (not sure if attachments are allowed on this
> mailing list) that does not make it possible for a local process anymore
> to replace canaries which in turn might disable SSP. Comments and/or
> review are much appreciated.
>
> The problem it is trying to solve is outlined in
> https://trac.torproject.org/13169#comment:4:
>
> (Quoting a cypherpunk)
>
> "In Windows you can to create any directories for any disks(C:, D:, ..
> Z:), only system directories (Windows directory, Program files, etc) are
> protected. Any process with privileges of any standard user can to
> create C:\dev\urandom file and to fill it by any stuff."
>
> And now the patch (arguably the subject can be a bit more elaborate):
Thanks for the patch, but it belongs to GCC and as such gcc-patches ML
is the right place to send it.
I have nothing to do with libssp nor have much experience with GCC
patches, but I have some suggestions:
> From 83e3ea38a5720df52bd5f78dcd3c7b7b842ddc3b Mon Sep 17 00:00:00 2001
> From: Erinn Clark
> Date: Wed, 12 Mar 2014 16:09:10 +0100
> Subject: [PATCH] skruffy patch
>
> ---
> libssp/ssp.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/libssp/ssp.c b/libssp/ssp.c
> index aaa5a32..37f4e27 100644
> --- a/libssp/ssp.c
> +++ b/libssp/ssp.c
> @@ -55,6 +55,7 @@ see the files COPYING3 and COPYING.RUNTIME
> respectively. If not, see
> /* Native win32 apps don't know about /dev/tty but can print directly
> to the console using "CONOUT$" */
> #if defined (_WIN32) && !defined (__CYGWIN__)
> +#include
> # define _PATH_TTY "CONOUT$"
> #else
> # define _PATH_TTY "/dev/tty"
> @@ -75,6 +76,20 @@ __guard_setup (void)
>if (__stack_chk_guard != 0)
> return;
>
> +#if defined (_WIN32) && !defined (__CYGWIN__)
> + HCRYPTPROV hprovider = 0;
Looking at the rest of the file, it seems that it maintains C89
compatibility or at least coding style uses declarations in the
beginning of the block.
> + if (CryptAcquireContext(&hprovider, NULL, NULL, PROV_RSA_FULL,
> + CRYPT_VERIFYCONTEXT | CRYPT_SILENT))
> +{
> + if (CryptGenRandom(hprovider, sizeof (__stack_chk_guard),
> + (BYTE *)&__stack_chk_guard) && __stack_chk_guard != 0)
> +{
> + CryptReleaseContext(hprovider, 0);
> + return;
> +}
> + CryptReleaseContext(hprovider, 0);
> +}
Did you consider using RtlGenRandom instead? This would simplify the code.
Another alternative would be using rand_s, which is a variant of rand
that doesn't require srand (see Wine implementation for an example [1]).
Its problem is, however, that it returns int, which is inconvenient for
64-bit arch in this case.
> +#else
>fd = open ("/dev/urandom", O_RDONLY);
>if (fd != -1)
> {
> @@ -85,6 +100,7 @@ __guard_setup (void)
> return;
> }
>
> +#endif
>/* If a random generator can't be used, the protector switches the guard
> to the "terminator canary". */
>p = (unsigned char *) &__stack_chk_guard;
> --
> 1.7.10.4
Thanks,
Jacek
[1] http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/msvcrt/misc.c#l68
--
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [Patch] - Hardening libssp against malicious local processes
Hi Jacek Jacek Caban: > Thanks for the patch, but it belongs to GCC and as such gcc-patches ML > is the right place to send it. > > I have nothing to do with libssp nor have much experience with GCC > patches, but I have some suggestions: thanks for both. I'll send it to gcc-patches to see what the GCC folks are thinking about it. Georg signature.asc Description: OpenPGP digital signature -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
