View Issue Details

IDProjectCategoryView StatusLast Update
0006153GNUnetutil librarypublic2020-04-22 17:27
ReporterfefeAssigned ToChristian Grothoff 
PrioritylowSeveritytrivialReproducibilityN/A
Status resolvedResolutionfixed 
Product VersionSVN HEAD 
Target Version0.13.0Fixed in Version 
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

View 2 revisions

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

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 => SVN HEAD
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 View Revisions
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