Received-SPF: pass (sog-mx-4.v43.ch3.sourceforge.com: domain of peernova.com
	designates 209.85.220.182 as permitted sender)
	client-ip=209.85.220.182; envelope-from=toshi@peernova.com;
	helo=mail-vc0-f182.google.com; 
MIME-Version: 1.0
Date: Thu, 29 May 2014 17:01:23 -0700
Message-ID: <CANFXpYMC2hYzFx5fPkqTv2LiCrtDny3yUV5u20ckK56zVjSpGA@mail.gmail.com>
From: Toshi Morita <toshi@peernova.com>
To: bitcoin-development@lists.sourceforge.net
Content-Type: multipart/alternative; boundary=047d7bd6c722ff8f3304fa92be9c
Subject: [Bitcoin-development] bitcoind minor bug in wallet and possible fix
Precedence: list

--047d7bd6c722ff8f3304fa92be9c
Content-Type: text/plain; charset=UTF-8

I ran bitcoind under valgrind and found a place where it references an
uninitialized variable in some cases:

tm@tm-VirtualBox:~/bitcoind/bitcoin/src$ valgrind ./bitcoind
==2337== Memcheck, a memory error detector
==2337== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==2337== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==2337== Command: ./bitcoind
==2337==
==2337== Conditional jump or move depends on uninitialised value(s)
==2337==    at 0x319176: CWallet::LoadKeyMetadata(CPubKey const&,
CKeyMetadata const&) (wallet.cpp:110)
==2337==    by 0x33645A: ReadKeyValue(CWallet*, CDataStream&, CDataStream&,
CWalletScanState&, std::string&, std::string&) (walletdb.cpp:509)
==2337==    by 0x3374F0: CWalletDB::LoadWallet(CWallet*) (walletdb.cpp:623)
==2337==    by 0x3218FD: CWallet::LoadWallet(bool&) (wallet.cpp:1485)
==2337==    by 0x157F16: AppInit2(boost::thread_group&) (init.cpp:958)
==2337==    by 0x140142: AppInit(int, char**) (bitcoind.cpp:143)
==2337==    by 0x13649E: main (bitcoind.cpp:180)
==2337==

The bug occurs here because nTimeFirstKey is not initialized when the
wallet is instantiated:

wallet.cpp:63
    if (!nTimeFirstKey || nCreationTime < nTimeFirstKey)
        nTimeFirstKey = nCreationTime;


I fixed it in my fork:

diff --git a/src/wallet.h b/src/wallet.h
index 9607415..b78045f 100644
--- a/src/wallet.h
+++ b/src/wallet.h
@@ -163,6 +163,7 @@ public:
         nOrderPosNext = 0;
         nNextResend = 0;
         nLastResend = 0;
+        nTimeFirstKey = 0;
     }

If this fix is ok please pull from my GitHub fork; username on GitHub is
tm314159.

Toshi

--047d7bd6c722ff8f3304fa92be9c
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div><div>I ran bitcoind under valgrind and found a place =
where it references an uninitialized variable in some cases:<br><br>tm@tm-V=
irtualBox:~/bitcoind/bitcoin/src$ valgrind ./bitcoind<br>=3D=3D2337=3D=3D M=
emcheck, a memory error detector<br>
=3D=3D2337=3D=3D Copyright (C) 2002-2012, and GNU GPL&#39;d, by Julian Sewa=
rd et al.<br>=3D=3D2337=3D=3D Using Valgrind-3.8.1 and LibVEX; rerun with -=
h for copyright info<br>=3D=3D2337=3D=3D Command: ./bitcoind<br>=3D=3D2337=
=3D=3D<br>=3D=3D2337=3D=3D Conditional jump or move depends on uninitialise=
d value(s)<br>
=3D=3D2337=3D=3D=C2=A0=C2=A0=C2=A0 at 0x319176: CWallet::LoadKeyMetadata(CP=
ubKey const&amp;, CKeyMetadata const&amp;) (wallet.cpp:110)<br>=3D=3D2337=
=3D=3D=C2=A0=C2=A0=C2=A0 by 0x33645A: ReadKeyValue(CWallet*, CDataStream&am=
p;, CDataStream&amp;, CWalletScanState&amp;, std::string&amp;, std::string&=
amp;) (walletdb.cpp:509)<br>
=3D=3D2337=3D=3D=C2=A0=C2=A0=C2=A0 by 0x3374F0: CWalletDB::LoadWallet(CWall=
et*) (walletdb.cpp:623)<br>=3D=3D2337=3D=3D=C2=A0=C2=A0=C2=A0 by 0x3218FD: =
CWallet::LoadWallet(bool&amp;) (wallet.cpp:1485)<br>=3D=3D2337=3D=3D=C2=A0=
=C2=A0=C2=A0 by 0x157F16: AppInit2(boost::thread_group&amp;) (init.cpp:958)=
<br>
=3D=3D2337=3D=3D=C2=A0=C2=A0=C2=A0 by 0x140142: AppInit(int, char**) (bitco=
ind.cpp:143)<br>=3D=3D2337=3D=3D=C2=A0=C2=A0=C2=A0 by 0x13649E: main (bitco=
ind.cpp:180)<br>=3D=3D2337=3D=3D<br><br></div>The bug occurs here because n=
TimeFirstKey is not initialized when the wallet is instantiated:<br>
<br>wallet.cpp:63<br>=C2=A0=C2=A0=C2=A0 if (!nTimeFirstKey || nCreationTime=
 &lt; nTimeFirstKey)<br>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 nTimeFir=
stKey =3D nCreationTime;<br><br><br></div><div>I fixed it in my fork:<br><b=
r>diff --git a/src/wallet.h b/src/wallet.h<br>index 9607415..b78045f 100644=
<br>
--- a/src/wallet.h<br>+++ b/src/wallet.h<br>@@ -163,6 +163,7 @@ public:<br>=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 nOrderPosNext =3D 0;<br>=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 nNextResend =3D 0;<br>=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 nLastResend =3D 0;<br>+=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 nTimeFirstKey =3D 0; <br>=C2=A0=C2=A0=
=C2=A0=C2=A0 }<br><br></div><div>
If this fix is ok please pull from my GitHub fork; username on GitHub is tm=
314159.<br><br>Toshi<br></div><div><br></div></div>

--047d7bd6c722ff8f3304fa92be9c--