[bitcoin-discuss] [bitcoin-dev] (no subject)

Matt Corallo lf-lists at mattcorallo.com
Sun Oct 16 21:54:07 UTC 2016


Moving to bitcoin-discuss since explaining C++ basics isnt relevant for
bitcoin-dev, nor is your continued trolling (I would ask that the mods
at least force the needlessly off-topic discussion to its own thread).

On 10/16/16 20:45, Tom Zander via bitcoin-dev wrote:
> On Sunday, 16 October 2016 19:35:52 CEST Matt Corallo wrote:
>> You keep calling flexible transactions "safer", and yet you haven't
>> mentioned that the current codebase is riddled with blatant and massive
>> security holes.
> 
> I am not afraid of people finding issues with my code, I'm only human. Would 
> appreciate you reporting actual issues instead of hinting at things here.
> Can't fix things otherwise :)

I highly recommend you go read that code as an exercise in reviewing
code, as the bugs are all relatively obvious and this is an important
skill if you wish to ship code responsible for billions of dollars of
other peoples' money (if you really want to cheat the line numbers are
further down in this email, but, really, take it as practice).

> But, glad you brought it up, the reason that FT is safer is because of the 
> amount of conceps that SegWit changes in a way that anyone doing development 
> on Bitcoin later will need to know about them in order to do proper 
> development.
> I counted 10 in my latest vlog entry.  FT only changes 2.
> 
> Its safer because its simpler.

I'm really not sure about that. In order to implement FT safely and
fully, you would need significantly more complication than segwit (I
encourage you to do so to prove me wrong, but all of the complication
with segwit - the network-object-request logic, fixing malleability, and
careful design thought, seemingly hasnt been handled at all with FT yet,
and it will have nearly identical issues). Further, your design
needlessly conflates several unrelated things, which is, in fact, /the/
design issue with Bitcoin which SegWit fixes, but which you have not
addressed: the wire/disk-encoding of data and consensus-calculation of
sighashes and txids are all entirely different concepts and do not need
to be handled identically. (OK, admittedly SegWit hasnt touched the idea
of changing the wire-encoding to be different from consensus yet, but we
should do that at some point going forward, and you seemingly are
excited by compression of such data - something we could easily accomplish).

>> For example, you seem to have misunderstood C++'s memory
>> model - you would have no less than three out-of-bound, probably
>> exploitable memory accesses in your 80-LoC deserialize method at
>> https://github.com/bitcoinclassic/bitcoinclassic/blob/develop/src/primitiv
>> es/transaction.cpp#L119 if you were to turn on flexible transactions (and
>> I only reviewed that method for 2 minutes). 
> 
> The unit test doesn't hit any of them. Valgrind only reports such possibly 
> exploitable issues in secp256k and CKey::MakeNewKey. The same as in Core.
> 
> I don't doubt that your 2 minute look shows stuff that others missed, and 
> that valgrind doesn't find either, but I'd be really grateful if you can 
> report them specifically to me in an email off list (or github, you know the 
> drill).
> More feedback will only help to make the proposal stronger and even better. 
> Thanks!

This is because of OpenSSL deliberately using un-initialized memory as
one of the many inputs it uses to seed its RNG - this is perfectly safe
within practical systems today, though is something we'd like to get
away from in the long-term (see the various work on building our own RNG
library).

However you seem to have completely misunderstood how valgrind (or
security, for that matter) works: valgrind will report use of memory in
an unsafe way in the codepath you took, it does not (and cannot) find
issues which are in a codepath you did not take. For example if someone
were to send a maliciously encoded object while the program is running
under valgrind, it would of course be reported, however this is not the
case if no such object is ever sent.

Specifically, your deserialization routine fails in a number of places
to check bounds before writing to memory (at least L138, L138 and L155),
which valgrind can only detect if you actually send an object which is
encoded in a malicious way. I highly recommend you look into tools like
afl-fuzz as well as seek additional code review before shipping code you
wish to be responsible for billions of dollars.


>> If you want to propose an
>> alternative to a community which has been in desperate need of fixes to
>> many problems for several years, please do so with something which would
>> not take at least a year to complete given a large team of qualified
>> developers.
> 
> I think FT fits the bill just fine :)  After your 2 minute look, take a bit 
> longer and check the rest of the code. You may be surprised with the 
> simplicity of the approach.

Actually quite the opposite, the approach you took here is much, much
more complicated than SegWit - you claim simplicity based on how short
the implementation is, however the implementation is filled with TODOs,
and does not begin to address the things that generated the vast
majority of SegWit's complexity - the networking and object-fetch logic.


More information about the bitcoin-discuss mailing list