View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0006153GNUnetutil librarypublic2020-04-03 12:142020-08-14 12:04
Reporterfefe Assigned ToChristian Grothoff  
PrioritylowSeveritytrivialReproducibilityN/A
Status closedResolutionfixed 
Product VersionGit master 
Target Version0.13.0 
Summary0006153: GNUNET_is_zero is very inefficient
DescriptionNot a security issue but it still bugs me enough to report it :-)

1108 #define GNUNET_is_zero(a) \
1109 ({ \
1110 static const typeof (*a) _z; \
1111 memcmp ((a), &_z, sizeof(_z)); \
1112 })

This will allocate static zero bytes for the sum of the sizes of all types it is ever called on. If it is called twice on the same type, it will allocate the zero bytes for that type twice, too. This is already bad, but it also means that memcmp will have to read two bytes of memory for each byte it wants to test for zero.
Steps To ReproduceRecommendation:

int is_zero(const void* a,size_t n) {
  size_t i;
  const char* b=a;
  for (i=0; i<n; ++i)
    if (b[i]) return 0;
  return 1;
}

#define GNUNET_is_zero(a) is_zero(a,sizeof(*a))
TagsNo tags attached.

Activities

Christian Grothoff

2020-04-03 14:27

manager   ~0015501

Two questions:
1) are you sure a modern C compiler doesn't do this as a compiler-optimization, and better than in your case because you call a function?
2) you were worried about constant-time for GNUNET_memcmp(), but here it's OK? Why?

fefe

2020-04-03 14:54

reporter   ~0015502

1) quite sure:

struct whatever {
  long a,b,c,d;
};

int foo(struct whatever* w) {
  return GNUNET_is_zero(w);
}

becomes

foo:
        .cfi_startproc
        movl $32, %edx
        movl $_z.2511, %esi
        jmp memcmp
        .cfi_endproc

2) the constant time problem is that you leak information about the comparant through the timing. When you compare against zero, then the comparant is not secret so we don't leak anything.

Christian Grothoff

2020-04-03 15:02

manager   ~0015503

Last edited: 2020-04-03 15:02

 And how are the public keys *secret*? (You would be right in the case of the GNU Name System where public keys can be secret, but for Taler I really don't see it.)

Christian Grothoff

2020-04-03 15:06

manager   ~0015504

Ok, C compilers are still more stupid than I thought. Will fix ;-).

Christian Grothoff

2020-04-03 15:10

manager   ~0015505

Fixed in 1b5dfc396..4e259dbbb

schanzen

2020-08-14 12:04

administrator   ~0016622

Closing as resolved since at least 0.13.2

Issue History

Date Modified Username Field Change
2020-04-03 12:14 fefe New Issue
2020-04-03 12:14 fefe Status new => assigned
2020-04-03 12:14 fefe Assigned To => Christian Grothoff
2020-04-03 14:25 Christian Grothoff Project Taler => GNUnet
2020-04-03 14:25 Christian Grothoff Category exchange => General
2020-04-03 14:27 Christian Grothoff Note Added: 0015501
2020-04-03 14:28 Christian Grothoff Category General => util library
2020-04-03 14:28 Christian Grothoff Product Version 0.7.0 => Git master
2020-04-03 14:54 fefe Note Added: 0015502
2020-04-03 15:02 Christian Grothoff Note Added: 0015503
2020-04-03 15:02 Christian Grothoff Note Edited: 0015503
2020-04-03 15:06 Christian Grothoff Note Added: 0015504
2020-04-03 15:10 Christian Grothoff Status assigned => resolved
2020-04-03 15:10 Christian Grothoff Resolution open => fixed
2020-04-03 15:10 Christian Grothoff Fixed in Version => 0.12.3
2020-04-03 15:10 Christian Grothoff Note Added: 0015505
2020-04-03 15:10 Christian Grothoff Target Version => 0.12.3
2020-04-22 17:27 schanzen Target Version 0.12.3 => 0.13.0
2020-06-01 00:52 Adminknox Issue cloned: 0006357
2020-08-14 12:04 schanzen Note Added: 0016622
2020-08-14 12:04 schanzen Status resolved => closed