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

Mon Jul 1 08:46:31 MST 2013

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01.07.2013 14:45, Mark Trompell wrote:
> At:
> 
> 248	250				pBuf++; 249	251				pBuf = strstr(pBuf, "<a
> href=\"");//Find the next link to a possible file name.
> 
> 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.

> 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:

https://gitorious.org/~jotik/sword-svn-mirrors/jotiks-sword-trunk/commit/fc85e83a00250a9d172bafc0dca33aa88c6e9e27

> 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-----