You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Küng <to...@gmail.com> on 2012/10/09 21:49:42 UTC
crash in latest release
Hi,
Just got the first crash report for the TSVN 1.7.10 release (svn 1.7.7).
Considering that the release is only an hour old, that indicates that I
will get a lot more of those...
Problem in libsvn_subr/win32_crypto.c, function
windows_password_decrypter(svn_boolean_t *done, ...):
...
if (!done)
return SVN_NO_ERROR;
now here the check is for the pointer, not the actual bool value.
And if it's set to false, the pointer check is still true and the code
goes on instead of returning here.
trunk and 1.7.x have the same bug, even though on trunk the code before
the if() is slightly different.
I think this needs to change to:
if (!*done)
return SVN_NO_ERROR;
I'm thinking of removing the latest TSVN release from the download
servers since this will crash every time a repository is accessed with
windows authentication - which most companies use.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
RE: crash in latest release
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: woensdag 10 oktober 2012 20:38
> To: Bert Huijben
> Cc: 'Johan Corveleyn'; dev@subversion.apache.org
> Subject: Re: crash in latest release
>
> On 10.10.2012 20:22, Bert Huijben wrote:
> >
> >
> >> -----Original Message-----
> >> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> >> Sent: woensdag 10 oktober 2012 18:40
> >> To: Johan Corveleyn
> >> Cc: dev@subversion.apache.org
> >> Subject: Re: crash in latest release
> >>
> >> On 09.10.2012 22:35, Johan Corveleyn wrote:
> >>
> >>> I used a release build (I always test with a release build), built
> >>> with Visual C Express 2008 on Windows XP. But indeed, it might depend
> >>> on the build ... I was probably lucky, or unlucky, depending on how
> >>> you look at it :-/ ...
> >>
> >> try this:
> >> $ svn log http://tortoisesvn.tigris.org/svn/tortoisesvn/trunk
> >> --username=guest
> >>
> >> this will trigger the crash.
> >
> > It doesn't trigger a crash for me. (Subversion 1.7.7 x64, SlikSvn).
> > I was also not able to crash an AnkhSVN which uses +- the same build on
> x86.
> >
> > Are you sure this is not specific to how TortoiseSVN builds Subversion or
> something like a password that was encrypted with a no longer valid key?
>
> Ah, now I got it:
> the crash only happens if you have saved the auth for the repo
> (tigris.org in the example above) with a different username as "guest".
>
> For example, I have saved the auth for tigris.org there with my username
> "steveking" so I get commit access.
>
> If I then pass --username=guest in a script because I only need read
> access, it crashes every time.
>
> So it's not as bad as I thought before.
> I guess it's only a bigger problem in case you use scripts which use
> different auth data.
Ok, with this new information (and a repository where I have access to two accounts) I can confirm that this crashes 'svn'.
I updated branches/1.7.x/STATUS in r1396944.
Bert
Re: crash in latest release
Posted by Stefan Küng <to...@gmail.com>.
On 10.10.2012 20:22, Bert Huijben wrote:
>
>
>> -----Original Message-----
>> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
>> Sent: woensdag 10 oktober 2012 18:40
>> To: Johan Corveleyn
>> Cc: dev@subversion.apache.org
>> Subject: Re: crash in latest release
>>
>> On 09.10.2012 22:35, Johan Corveleyn wrote:
>>
>>> I used a release build (I always test with a release build), built
>>> with Visual C Express 2008 on Windows XP. But indeed, it might depend
>>> on the build ... I was probably lucky, or unlucky, depending on how
>>> you look at it :-/ ...
>>
>> try this:
>> $ svn log http://tortoisesvn.tigris.org/svn/tortoisesvn/trunk
>> --username=guest
>>
>> this will trigger the crash.
>
> It doesn't trigger a crash for me. (Subversion 1.7.7 x64, SlikSvn).
> I was also not able to crash an AnkhSVN which uses +- the same build on x86.
>
> Are you sure this is not specific to how TortoiseSVN builds Subversion or something like a password that was encrypted with a no longer valid key?
Ah, now I got it:
the crash only happens if you have saved the auth for the repo
(tigris.org in the example above) with a different username as "guest".
For example, I have saved the auth for tigris.org there with my username
"steveking" so I get commit access.
If I then pass --username=guest in a script because I only need read
access, it crashes every time.
So it's not as bad as I thought before.
I guess it's only a bigger problem in case you use scripts which use
different auth data.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
RE: crash in latest release
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: woensdag 10 oktober 2012 18:40
> To: Johan Corveleyn
> Cc: dev@subversion.apache.org
> Subject: Re: crash in latest release
>
> On 09.10.2012 22:35, Johan Corveleyn wrote:
>
> > I used a release build (I always test with a release build), built
> > with Visual C Express 2008 on Windows XP. But indeed, it might depend
> > on the build ... I was probably lucky, or unlucky, depending on how
> > you look at it :-/ ...
>
> try this:
> $ svn log http://tortoisesvn.tigris.org/svn/tortoisesvn/trunk
> --username=guest
>
> this will trigger the crash.
It doesn't trigger a crash for me. (Subversion 1.7.7 x64, SlikSvn).
I was also not able to crash an AnkhSVN which uses +- the same build on x86.
Are you sure this is not specific to how TortoiseSVN builds Subversion or something like a password that was encrypted with a no longer valid key?
Bert
Re: crash in latest release
Posted by Stefan Küng <to...@gmail.com>.
On 09.10.2012 22:35, Johan Corveleyn wrote:
> I used a release build (I always test with a release build), built
> with Visual C Express 2008 on Windows XP. But indeed, it might depend
> on the build ... I was probably lucky, or unlucky, depending on how
> you look at it :-/ ...
try this:
$ svn log http://tortoisesvn.tigris.org/svn/tortoisesvn/trunk
--username=guest
this will trigger the crash.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: crash in latest release
Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, Oct 9, 2012 at 10:30 PM, Stefan Küng <to...@gmail.com> wrote:
> On 09.10.2012 22:25, Johan Corveleyn wrote:
>>
>> On Tue, Oct 9, 2012 at 10:15 PM, Branko Čibej <br...@wandisco.com> wrote:
>>>
>>> On 09.10.2012 21:49, Stefan Küng wrote:
>>>>
>>>> Hi,
>>>>
>>>> Just got the first crash report for the TSVN 1.7.10 release (svn 1.7.7).
>>>> Considering that the release is only an hour old, that indicates that
>>>> I will get a lot more of those...
>>>>
>>>> Problem in libsvn_subr/win32_crypto.c, function
>>>> windows_password_decrypter(svn_boolean_t *done, ...):
>>>>
>>>> ...
>>>> if (!done)
>>>> return SVN_NO_ERROR;
>>>>
>>>>
>>>> now here the check is for the pointer, not the actual bool value.
>>>> And if it's set to false, the pointer check is still true and the code
>>>> goes on instead of returning here.
>>>>
>>>> trunk and 1.7.x have the same bug, even though on trunk the code
>>>> before the if() is slightly different.
>>>>
>>>> I think this needs to change to:
>>>>
>>>> if (!*done)
>>>> return SVN_NO_ERROR;
>>>
>>>
>>> You're right. Fixed in r1396285 but I don't have a way to test it.
>>>
>>> I don't know why our test suite didn't catch this; it should trigger
>>> every time an encrypted password is read from disk on Windows. But maybe
>>> the testsuite doesn't even test that.
>>
>>
>> I doubt that it can be triggered by simply reading an encrypted
>> password from %APPDATA%\Subversion\auth\svn.simple, if that's what you
>> mean. Otherwise I would have definitely run into the crash (if not
>> during the testsuite, I would have seen it during my manual testing of
>> serf checkouts/exports, on my work pc).
>
>
> Not necessarily: in my debug builds, it doesn't crash. Only the release
> builds crash because the strlen() there accesses an invalid pointer, while
> in the debug build the char* gets a 'initialized' to a valid string.
> depending on your build, the
>
> char *in;
>
> might point to some valid memory and therefore not crash (as it does in my
> debug builds).
I used a release build (I always test with a release build), built
with Visual C Express 2008 on Windows XP. But indeed, it might depend
on the build ... I was probably lucky, or unlucky, depending on how
you look at it :-/ ...
--
Johan
Re: crash in latest release
Posted by Stefan Küng <to...@gmail.com>.
On 09.10.2012 22:25, Johan Corveleyn wrote:
> On Tue, Oct 9, 2012 at 10:15 PM, Branko Čibej <br...@wandisco.com> wrote:
>> On 09.10.2012 21:49, Stefan Küng wrote:
>>> Hi,
>>>
>>> Just got the first crash report for the TSVN 1.7.10 release (svn 1.7.7).
>>> Considering that the release is only an hour old, that indicates that
>>> I will get a lot more of those...
>>>
>>> Problem in libsvn_subr/win32_crypto.c, function
>>> windows_password_decrypter(svn_boolean_t *done, ...):
>>>
>>> ...
>>> if (!done)
>>> return SVN_NO_ERROR;
>>>
>>>
>>> now here the check is for the pointer, not the actual bool value.
>>> And if it's set to false, the pointer check is still true and the code
>>> goes on instead of returning here.
>>>
>>> trunk and 1.7.x have the same bug, even though on trunk the code
>>> before the if() is slightly different.
>>>
>>> I think this needs to change to:
>>>
>>> if (!*done)
>>> return SVN_NO_ERROR;
>>
>> You're right. Fixed in r1396285 but I don't have a way to test it.
>>
>> I don't know why our test suite didn't catch this; it should trigger
>> every time an encrypted password is read from disk on Windows. But maybe
>> the testsuite doesn't even test that.
>
> I doubt that it can be triggered by simply reading an encrypted
> password from %APPDATA%\Subversion\auth\svn.simple, if that's what you
> mean. Otherwise I would have definitely run into the crash (if not
> during the testsuite, I would have seen it during my manual testing of
> serf checkouts/exports, on my work pc).
Not necessarily: in my debug builds, it doesn't crash. Only the release
builds crash because the strlen() there accesses an invalid pointer,
while in the debug build the char* gets a 'initialized' to a valid string.
depending on your build, the
char *in;
might point to some valid memory and therefore not crash (as it does in
my debug builds).
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: crash in latest release
Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, Oct 9, 2012 at 10:15 PM, Branko Čibej <br...@wandisco.com> wrote:
> On 09.10.2012 21:49, Stefan Küng wrote:
>> Hi,
>>
>> Just got the first crash report for the TSVN 1.7.10 release (svn 1.7.7).
>> Considering that the release is only an hour old, that indicates that
>> I will get a lot more of those...
>>
>> Problem in libsvn_subr/win32_crypto.c, function
>> windows_password_decrypter(svn_boolean_t *done, ...):
>>
>> ...
>> if (!done)
>> return SVN_NO_ERROR;
>>
>>
>> now here the check is for the pointer, not the actual bool value.
>> And if it's set to false, the pointer check is still true and the code
>> goes on instead of returning here.
>>
>> trunk and 1.7.x have the same bug, even though on trunk the code
>> before the if() is slightly different.
>>
>> I think this needs to change to:
>>
>> if (!*done)
>> return SVN_NO_ERROR;
>
> You're right. Fixed in r1396285 but I don't have a way to test it.
>
> I don't know why our test suite didn't catch this; it should trigger
> every time an encrypted password is read from disk on Windows. But maybe
> the testsuite doesn't even test that.
I doubt that it can be triggered by simply reading an encrypted
password from %APPDATA%\Subversion\auth\svn.simple, if that's what you
mean. Otherwise I would have definitely run into the crash (if not
during the testsuite, I would have seen it during my manual testing of
serf checkouts/exports, on my work pc).
--
Johan
Re: crash in latest release
Posted by Stefan Küng <to...@gmail.com>.
On 09.10.2012 22:15, Branko Čibej wrote:
> On 09.10.2012 21:49, Stefan Küng wrote:
>> Hi,
>>
>> Just got the first crash report for the TSVN 1.7.10 release (svn 1.7.7).
>> Considering that the release is only an hour old, that indicates that
>> I will get a lot more of those...
>>
>> Problem in libsvn_subr/win32_crypto.c, function
>> windows_password_decrypter(svn_boolean_t *done, ...):
>>
>> ...
>> if (!done)
>> return SVN_NO_ERROR;
>>
>>
>> now here the check is for the pointer, not the actual bool value.
>> And if it's set to false, the pointer check is still true and the code
>> goes on instead of returning here.
>>
>> trunk and 1.7.x have the same bug, even though on trunk the code
>> before the if() is slightly different.
>>
>> I think this needs to change to:
>>
>> if (!*done)
>> return SVN_NO_ERROR;
>
> You're right. Fixed in r1396285 but I don't have a way to test it.
>
> I don't know why our test suite didn't catch this; it should trigger
> every time an encrypted password is read from disk on Windows. But maybe
> the testsuite doesn't even test that.
>
> Stefan, if you can verify that the fix works, I'll propose it for
> backport to 1.7. I think, for a crash as basic as this one, we should
> probably roll 1.7.8 as soon as the fix is confirmed.
Thanks for the quick fix.
Verified the fix on the 1.7.x branch: works fine now.
I don't have a test build ready for trunk though, but since this fixes
the problem on 1.7.x I guess it's fine on trunk as well.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: crash in latest release
Posted by Branko Čibej <br...@wandisco.com>.
On 09.10.2012 21:49, Stefan Küng wrote:
> Hi,
>
> Just got the first crash report for the TSVN 1.7.10 release (svn 1.7.7).
> Considering that the release is only an hour old, that indicates that
> I will get a lot more of those...
>
> Problem in libsvn_subr/win32_crypto.c, function
> windows_password_decrypter(svn_boolean_t *done, ...):
>
> ...
> if (!done)
> return SVN_NO_ERROR;
>
>
> now here the check is for the pointer, not the actual bool value.
> And if it's set to false, the pointer check is still true and the code
> goes on instead of returning here.
>
> trunk and 1.7.x have the same bug, even though on trunk the code
> before the if() is slightly different.
>
> I think this needs to change to:
>
> if (!*done)
> return SVN_NO_ERROR;
You're right. Fixed in r1396285 but I don't have a way to test it.
I don't know why our test suite didn't catch this; it should trigger
every time an encrypted password is read from disk on Windows. But maybe
the testsuite doesn't even test that.
Stefan, if you can verify that the fix works, I'll propose it for
backport to 1.7. I think, for a crash as basic as this one, we should
probably roll 1.7.8 as soon as the fix is confirmed.
-- Brane
> I'm thinking of removing the latest TSVN release from the download
> servers since this will crash every time a repository is accessed with
> windows authentication - which most companies use.
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download