[sword-devel] installmgr (and xiphos) crashes (svn 2831)

Mon Jul 1 23:45:34 MST 2013

On Mon, Jul 1, 2013 at 5:46 PM, Jaak Ristioja <jaak at ristioja.ee> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 01.07.2013 14:45, Mark Trompell wrote:

>> how do we know that pBuf++ is actually not outside our buffer?
>
> You mean pBuf before pBuf = strstr(pBuf, "<a href=\"") ? Because it
> points past the last double quote found in a \0-terminated string.

Okay, got that.

>> btw, why abort if pBufRes > pBuf?
>
> I don't understand your question, but this did help me find a bug in
> my patch. Here's an amendment:

Question is how can pBufRes get <= pBuf, if the char is found it is >=
pBuf or NULL if char isn't found.
The only reason for pBuf==pBufRes I can imagine is a <a href=""> which
is annoying but not really a reason for aborting, which is what assert
does in case the assertion fails.
We might still want to use the other filenames found.

>> why not something like probably even uglier attached patch? I want
>> to get deeper inside C and C++ so I want to understand.
>>
>> On Thu, Jun 27, 2013 at 10:33 PM, Jaak Ristioja <jaak at ristioja.ee>
>> wrote: Patch for pointer dereference issue:
>>
>>
>> https://gitorious.org/~jotik/sword-svn-mirrors/jotiks-sword-trunk/commit/1b8ab91ff994c8584d6c61cb7d334273732d8216
>>
>>  Patch for buffer overflow:
>>
>>
>> https://gitorious.org/~jotik/sword-svn-mirrors/jotiks-sword-trunk/commit/4a261b27a7bec9d9300da6c357666a3851f3d34e
>>
>>  There you go! Took me half an hour.
>>
>> Blessings, Jaak
>>
>> On 27.06.2013 22:41, Mark Trompell wrote:
>>>>> I see. I'll try to come up with a better patch on Monday. I
>>>>> won't have time earlier. Blessings Mark --- Ursprüngl.
>>>>> Mitteilung --- Von: Jaak Ristioja Gesend.:  27.06.2013, 16:15
>>>>> An: sword-devel at crosswire.org Betreff: Re: [sword-devel]
>>>>> installmgr (and xiphos) crashes (svn 2831)
>>>>>
>>>>>
>>>>> I think you only fixed pBuf not being set to NULL
>>>>> prematurely. But this:
>>>>>
>>>>> memset(possibleName, 0, 400);
>>>>>
>>>>> doesn't help. The sprintf function always writes a
>>>>> terminating \0 character. The problem is not that a \0
>>>>> character is not written, because it is written (unless a
>>>>> memory error occurs first). The problem is that if
>>>>> possibleNameLength > 399 then it writes the characters
>>>>> (including the terminating \0 character) past the end of the
>>>>> possibleName buffer, corrupting memory, potentially outside
>>>>> of the virtual address space of the program (usually
>>>>> triggering the OS to kill the process with a segfault or
>>>>> something).
>>>>>
>>>>> The memset call is not needed, but it should be checked that
>>>>> possibleNameLength < 400 (strictly "less-than"). Otherwise
>>>>>
>>>>> sprintf(possibleName, "%.*s", possibleNameLength, pBuf);
>>>>>
>>>>> is a security vulnerability. I wonder whether a CVE is
>>>>> required.
>>>>>
>>>>>
>>>>> Blessings, Jaak
>>>>>
>>>>> On 27.06.2013 14:45, Mark Trompell wrote:
>>>>>> Sending again with tabs instead of blancs in the first
>>>>>> hunk
>>>>>
>>>>>> On Thu, Jun 27, 2013 at 1:17 PM, Mark Trompell
>>>>>> <mark at foresightlinux.org> wrote:
>>>>>>> I just fixed it :). Attached patch will initialize
>>>>>>> possibleNames with 0 bytes to make sure we always have
>>>>>>> the name 0 terminated properly. and it will move the
>>>>>>> pBuf=pBufRes into the check for ifBufRes != NULL, in case
>>>>>>> no filesize is found (because of another apache is
>>>>>>> displaying it differently) Shouldn't break existing
>>>>>>> setups.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> _______________________________________________
>>>>>> sword-devel mailing list: sword-devel at crosswire.org
>>>>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>>>>> Instructions to unsubscribe/change your settings at above
>>>>>> page
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________ sword-devel
>>>>> mailing list: sword-devel at crosswire.org
>>>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>>>> Instructions to unsubscribe/change your settings at above
>>>>> page
>>>>>
>>>>>
>>>>> _______________________________________________ sword-devel
>>>>> mailing list: sword-devel at crosswire.org
>>>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>>>> Instructions to unsubscribe/change your settings at above
>>>>> page
>>>>>
>>
>>>
>>> _______________________________________________ sword-devel
>>> mailing list: sword-devel at crosswire.org
>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>> Instructions to unsubscribe/change your settings at above page
>>
>>
>>
>>
>>
>> _______________________________________________ sword-devel mailing
>> list: sword-devel at crosswire.org
>> http://www.crosswire.org/mailman/listinfo/sword-devel Instructions
>> to unsubscribe/change your settings at above page
>>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.20 (GNU/Linux)
>
> iQgcBAEBAgAGBQJR0aRTAAoJEEqsYmEt1rCObWtAAMZGEzsbD5kLFfygnT3nYmWG
> W22SM41J1hcorszkFoeu5d2AE9X1J8jRWq6rZ7wwh2OpSxCgyp/UiW/AghRlZrw6
> xkzhsAOKR1CvQVs4ILYcfM0ODwYgCTvxrJ8YGHVpW/Qadr0Ld9moXPEmHTHPqDYv
> 5GHqOzdZ9UlWnuRSBgPiiCIXV3eqzrvgXOGTm8dBneYGY0wioSXah+Ps6SreT1Ik
> YRmJBK+vlcDi50hoDbnsQ42AT4Ou3YijF+SDVh8sCa0GYb33iMfny2uedVLKl67G
> grhlLLkLS5Z9iPx+ZyTUCeGkTmvAY21wyvJgxsiTPqEVWPmLKnyz2u7SlfZOGeWa
> u+C69pz3hHYiZO5bhRPL0vvh8voXhPWyHXY3Us5pQvsCF33ShgfHcVcp9UrpF/mz
> D+WGEANOPEZq6/cLUg2haLHy83xKNLA8lVT1j5bLuhEFkS1ung/MRCesVTr68QxE
> twAmwJXzev5EhejYBqHMlPWlfKxsmDYgTNGJ3rkg84V4Eg192ORrt217Y242HJiY
> H4qxBjWSEfHZNEc/pWqStoJpEyCos+PyamgKLoQljyVOEd/iTzfJUM9EmwnmFU1Z
> QzVzj2tcf6nksX9uP5e1AtuK6mALpdiLis8l6R2elnXLgv9NHwq7gwnIaHLXx+2W
> JSgnk0I1B/Y0VFv5AjvDQcCXmK/9vp+PINsQsiNf/FQIzFyeZ3viHmGud7Di+viE
> nd8oOLz0fypSn9qC3g2Ovt553SxDgiZRbsmjSjcfaogLznFpauk723gaJDlMtjnx
> df0MINi1KVU91Fw0GVfk1mIaJs0YnxjK7MPKTRwznFAe0nGMevD2c64/mXH5prQ2
> E1fqn1hW2M7Dv0ogITtJtPJvkuuKxrKm+WV7iucL1n+enIcBggbEgCvBJaXhEJgg
> SeecOhrPTSUZusHWRwq2DPqWCtD2ZtaZqpHr3sk6KInHIRggGqznAuaC4/I1vY/k
> +hyKPlPmmlRaaL/MoIOD5HUDbamRaLGf63JNhUwcD5xQnrB4ENLmL0YIiyyP0CXm
> 81wFC+UyPQHBdP2JUhpM+LCHPzJWfkzh+mE4UhnXFJd+wO6bOlg/wE53xpF0gULJ
> NAyinINZ1OVDplJeT8MfQjzG1PyJhYvSKyolFEhgSoMVCyNK9BKPlFByGyE/R+qh
> ko93S7epKQrJuNX4b3ueULEMctk3Cc9oFlgMoK0aeDkl+JOSvRSYnf+VbE+qEtkt
> mar6rsckmtmS2rEbtJOS4oRMzxCl5fy/umBxObXCrvUuo5/o/a/40Y/OX2h8SHqi
> r3JlhFlZu7bllt3E+GCN2ZHu+nKXRRPkNKVkPHLNkI2VnewXn4wOmLorGs3hS7Vb
> GaOcuDxZTMGxBd00LuPjPLfCA+p9UYQkzU/uUsGBDa5oaz0hiBD8nBvrz21Obc3/
> gbJwfF2/QHbVyPQJfmwTTh/1ttwuE5UC5A7/tidQNVSp478WKMJOUvBrCMf6ZmW9
> EudcpP47qQCfJahdWWyFFzPUFm3G2qWl8knjRuPvY6VHLDstukKhspN79LpnM2mD
> d62o3OcnMoKRLTfAE0/MIO7UDnOdrmtzO8hzt5s4F/LOqIuF2P/+1R6TYhwzmsad
> YPPlW5RAq/OYFQ6LOsjeocvK7JFN936sw3LitLueiaa5PoLf9DdorzV+RzeZajfV
> hOaxQgUAgm7HE8egHWOO4ukqVNouHasKdxMpT9S/AsOxcXYa6PehxUB43e46xTPS
> qRm3Veby/NhdxmsCtWg8t/EkUPoR38Hm0yEbxLrIe9VFGKbsAoRulR65qGMAE1QW
> HjGT6vowbg1BxW3ADA21G7upmdmktZzKoVcsWjEvPABQZC3NBbO1tPRGVhz3OWuE
> IjlJ9ELB5xjghp/3gyZpfi7TOhSV9qis6PKZSE8g8sBZm3ucxcCTOQtAHvIHPzaP
> U4FYUd4cKlOuR0Rdbv1LIc5iJwV3/dA7goj2nsQL7oi1YL8pF8sFU78P3b+wCyYo
> 7kCIJv/TIgOsxfnkFWuaZ85zo9XCjnjrLU3cHYCoNyD1+lXfcSkh7GsTMYPSA94S
> 4AHBnvSU28CIp0RN0KBnP8RQPFxmrCcxaltE9XczDwt7VHEohlDfbbcz5xA3JLN9
> Ti4kKVh9PDnZhNlKoSKDOLoLhn3QmsY+bEcwd8tgl/sKznDbP9GWFHgZQq0hOkl3
> WmcchQSuWWC27h3VDQES16gTEryQDpnVkJWqQJi7vuZFtgcu9i6lgIDRKcVQbC2A
> LBz0LmOWvI3v4XNSAfEdaS2ALvGTbDmy2jLCY2p748CRk2tG3LVVZvEy4NLtIpJC
> vb6VhfwQlCc7tL8Ib9arHtAimQ7155fj+2mLV8HFRycrpP1naHSb1rIJagoWBbQH
> jvQrXTpztsKzm2svYAs36wDHVM/uVtk4k/8rF2kkquThO00ID/wQ/2t9i6hNWj0/
> sn11PN2UZH4WdSvR5PrwbvzNyb/3zztlcEDLtMOKVLtu60dQt7jVkjyiGK+FECEj
> Ai+JnTcOo+5J7sUqWhPA6t3K3eqXcPJuFtEVOfkPmR//ibwqCbYGwB5PxFn/Ki6p
> XNY7XTliEMF+y6VOZMhcwrEWCJCMOQH3xTXzWZYVBbA6BIF++yD25ktWeK70K6EA
> IoSsjupW7DofUqrReDahGYh4d8Jv9tRWJvEu6pLxy7dMGw8RjciebUvQKK0P5Aoq
> VSIvQ+cbuAe8jkdMhvjp
> =VHBE
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> sword-devel mailing list: sword-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page

-- 
Mark Trompell

Foresight Linux Xfce Edition
Cause your desktop should be freaking cool
(and Xfce)