Opened 10 years ago

Last modified 7 years ago

#1254 closed defect (Fixed)

supply correct type to sizeof

Reported by: ekir Owned by:
Priority: Low Milestone:
Component: Core Tor/Tor Version: 0.2.2.7-alpha
Severity: Keywords:
Cc: ekir, Sebastian, arma, nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

in aes.c

271c271
< memset(cipher, 0, sizeof(cipher));
---

memset(cipher, 0, sizeof(aes_cnt_cipher_t));

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Change History (13)

comment:1 Changed 10 years ago by arma

Thanks!

Interesting. It looks like it's not the end of the world -- we're trying to
zero it out before freeing it, and only partially zeroing it out.

(I think the struct is bigger than a pointer on all platforms -- right?)

Clearly something that should go into 0.2.2.x.

comment:2 Changed 10 years ago by ekir

The struct is much bigger on my platform so most of it stays nonzeroed. It would be good to zero all of it I think.

comment:3 Changed 10 years ago by Sebastian

See branch bug1254 in my repo (
http://gitweb.torproject.org//sebastian/tor.git?a=commitdiff;h=f5112fa48754b33aa46c1bf79e11cf400d5084a8
). Does that work for you? Do you want other kinds of credit?

comment:4 Changed 10 years ago by arma

nickm committed this patch today, and it will get into 0.2.2.9-alpha

comment:5 Changed 10 years ago by arma

Parakeep on irc convinced me this should get backported to 0.2.1.25.

That means we should also tell weasel about it for his lenny 0.2.0.x.

"no need cold boot. just another user who malloced many memory. no need any privs."

yuck.

comment:6 Changed 10 years ago by nickm

To be clear, another user who "malloced many menory" can maybe force the key to get swapped out. But such
a user can do it anyway; he just needs to make the attempt before the key is freed. This patch only shortens
the number of keys you can get by looking at an unencrypted swap from a Tor process to those that were
actually in use when the process swapped.

comment:7 Changed 10 years ago by ekir

Sebastian Hahn - Monday, 22 Feb 2010, 11:46am
Does that work for you?

yes

Do you want other kinds of credit?

No. Thanks for quick patching.

Nick Mathewson - Tuesday, 23 Feb 2010, 12:45am
To be clear, another user who "malloced many menory" can maybe force the key to get swapped out. But such
a user can do it anyway; he just needs to make the attempt before the key is freed.


Swapped to disk? I thought more important is that other malloc call may return memory with some
parts of key in it? That is what worried me, but maybe can't happen, or only if you already control
tor process?

More generally memset(pointer, 0, sizeof(pointer)) can indicate a bug. You like to zero out what the pointer
points to and not just pointer. That's how I found this.

comment:8 Changed 10 years ago by nickm

Kernels zero out ram before they hand it out to a process; each process has its own heap. If I could see another
process's freed RAM by calling malloc(), that would a security hole in the OS.

comment:9 Changed 10 years ago by ekir

Kernels zero out ram before they hand it out to a process

Ok, of course. Thanks for your response.

comment:10 Changed 10 years ago by nickm

I'm told that not every OS in the world zeroes out RAM correctly. The notable ones that didn't are Win95 and earlier,
and Mac OS 9.

Tor doesn't work on Mac OS 9, and probably not on win95 either. Even if it did, trying to keep Tor secure on a

win95 box in the presence of hostile processes is probably not a viable exercise.

comment:11 Changed 10 years ago by PerFor

Zeroing of Tor memory is the one of conditions for success of
Perfect Forward Secrecy.

Zeroing during the allocation may named of security measure but it does
not guarantee the success of PFS. Zeroing an allocated memory does not
guarantee that a during of interval between a release of memory and its
allocation to another process will not happened unauthorized access
to that memory (direct access to the equipment will still be relevant).

Hence is not a zero probability of getting access to a sessions keys,
even after a considerable time as you thinked a keys "was" erased.

Moreover for Linux kernel can be disabled the zeroing of memory for
the case of performance vs. security:
http://mailman.uclinux.org/pipermail/uclinux-dev/2009-October/001523.html

comment:12 Changed 9 years ago by arma

flyspray2trac: bug closed.
fixed in 0.2.1.25 and 0.2.2.9-alpha

comment:13 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.