From e96a720f0463648df815a22a042889c361defa9f Mon Sep 17 00:00:00 2001 From: Erik Bray Date: Thu, 26 Jan 2017 20:48:15 +0100 Subject: [PATCH 1/2] Fixes a serious bug in Random.byte Python's bytecode compiler has a peephole optimizer which, among other things, can recognize constant expressions and replace them with a constant. In `Random.byte` the expression `t2b('\0')` is recognized as a constant and is replaced with a single constant compiled into the function's bytecode. This means that every time you run `Random.byte`, rather than creating a new `str` object (or `bytes` in Python 3) it's reusing the same one each time, and `wc_RNG_GenerateByte` is writing right into that constant object's buffer; hence the following behavior: ``` In [55]: rng = Random() In [56]: a = rng.byte() In [57]: a Out[57]: "'" In [58]: rng.byte() Out[58]: '\x11' In [59]: a Out[59]: '\x11' In [60]: rng.byte() Out[60]: '\x16' In [61]: a Out[61]: '\x16' In [62]: rng.byte.__func__.__code__.co_consts Out[62]: ('\n Generate and return a random byte.\n ', '\x16', 0, 'RNG generate byte error (%d)') In [63]: rng.byte() Out[63]: '\xad' In [64]: rng.byte.__func__.__code__.co_consts Out[64]: ('\n Generate and return a random byte.\n ', '\xad', 0, 'RNG generate byte error (%d)') ``` `Random.bytes` does not necessarily have this problem since its result buffer is not a constant expression, though I feel like it could also in principle be affected if the string were interned (though I couldn't produce such a result). Nevertheless, it doesn't seem like a good idea to be updating `str` objects' buffers directly. --- wrapper/python/wolfcrypt/wolfcrypt/random.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/wrapper/python/wolfcrypt/wolfcrypt/random.py b/wrapper/python/wolfcrypt/wolfcrypt/random.py index f819f51d8..eebebc41f 100644 --- a/wrapper/python/wolfcrypt/wolfcrypt/random.py +++ b/wrapper/python/wolfcrypt/wolfcrypt/random.py @@ -46,23 +46,23 @@ class Random(object): """ Generate and return a random byte. """ - result = t2b("\0") + result = _ffi.new('byte[1]') ret = _lib.wc_RNG_GenerateByte(self.native_object, result) if ret < 0: raise WolfCryptError("RNG generate byte error (%d)" % ret) - return result + return _ffi.string(result, 1) def bytes(self, length): """ Generate and return a random sequence of length bytes. """ - result = t2b("\0" * length) + result = _ffi.new('byte[%d]' % length) ret = _lib.wc_RNG_GenerateBlock(self.native_object, result, length) if ret < 0: raise WolfCryptError("RNG generate block error (%d)" % ret) - return result + return _ffi.string(result, length) From a094a36fa8518889f46a704a55b4cfe7efec1ef7 Mon Sep 17 00:00:00 2001 From: Erik Bray Date: Sat, 28 Jan 2017 15:55:42 +0100 Subject: [PATCH 2/2] Update random.py Realized that `ffi.string()` could truncate the output on null bytes. --- wrapper/python/wolfcrypt/wolfcrypt/random.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wrapper/python/wolfcrypt/wolfcrypt/random.py b/wrapper/python/wolfcrypt/wolfcrypt/random.py index eebebc41f..e034f5c58 100644 --- a/wrapper/python/wolfcrypt/wolfcrypt/random.py +++ b/wrapper/python/wolfcrypt/wolfcrypt/random.py @@ -52,7 +52,7 @@ class Random(object): if ret < 0: raise WolfCryptError("RNG generate byte error (%d)" % ret) - return _ffi.string(result, 1) + return _ffi.buffer(result, 1)[:] def bytes(self, length): @@ -65,4 +65,4 @@ class Random(object): if ret < 0: raise WolfCryptError("RNG generate block error (%d)" % ret) - return _ffi.string(result, length) + return _ffi.buffer(result, length)[:]