Bug 189296 - cscope buffer overflow (includes patch)
Summary: cscope buffer overflow (includes patch)
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: cscope   
(Show other bugs)
Version: 5
Hardware: All Linux
medium
high
Target Milestone: ---
Assignee: Neil Horman
QA Contact:
URL:
Whiteboard: bzcl34nup
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2006-04-18 21:26 UTC by Ronald Wahl
Modified: 2008-04-11 00:00 UTC (History)
1 user (show)

Fixed In Version: 15.6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-04-11 00:00:02 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Fix for the buffer overflow (524 bytes, patch)
2006-04-18 21:26 UTC, Ronald Wahl
no flags Details | Diff
alternate patch to fix buffer overflow (336 bytes, patch)
2006-04-19 13:22 UTC, Neil Horman
no flags Details | Diff
Revised Patch (369 bytes, patch)
2006-04-19 17:13 UTC, Ronald Wahl
no flags Details | Diff

Description Ronald Wahl 2006-04-18 21:26:51 UTC
Description of problem:

"cscope -R -q" exits on my local source repository and reports an buffer overflow.

Version-Release number of selected component (if applicable):

15.5

How reproducible:

Always for me. Unfortunately I cannot release my repository to others. 

Steps to Reproduce:

call cscope -R -q
  
Actual results:

Buffer overflow report.

Expected results:

A running cscope waiting for user input.

Additional info:

Backtrace (partly useful)

#0  0x00417402 in __kernel_vsyscall ()
(gdb) bt
#0  0x00417402 in __kernel_vsyscall ()
#1  0x00ccd159 in *__GI_raise (sig=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0x00cce6e3 in *__GI_abort () at abort.c:88
#3  0x00d01a1b in __libc_message (do_abort=2, 
    fmt=0xdbf444 "*** buffer overflow detected ***: %s terminated\n")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:170
#4  0x00d80965 in *__GI___chk_fail () at chk_fail.c:31
#5  0x00d7ff07 in __strcpy_chk (
    dest=0x80a4c00
"a1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"...,

    src=0xbf9f8b90
"a1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"...,

    destlen=4294967295) at strcpy_chk.c:61
#6  0x08059d1f in invmake (invname=0x9ac5130 "ncscope.in.out", 
    invpost=0x9ac5148 "ncscope.po.out", infile=0x9ab1fe0) at invlib.c:220
#7  0x0804f25d in build () at build.c:452
#8  0x0805b1b5 in main (argc=0, argv=0xbf9f97fc) at main.c:560
#9  0x00cba7e4 in __libc_start_main (main=0x805a730 <main>, argc=3, 
    ubp_av=0xbf9f97f4, init=0x805cbb0 <__libc_csu_init>, 
    fini=0x805cba8 <__libc_csu_fini>, rtld_fini=0x425e40 <_dl_fini>, 
    stack_end=0xbf9f97ec) at libc-start.c:231
#10 0x0804a031 in _start ()

The fix:

The problematic code is in invlib.c line 220. It contais a strcpy() which copies
a buffer that might be up to 1000 bytes in length into a buffer that is only 512
bytes large. I attach a quick fix which hopefully doesn't break other things.

Comment 1 Ronald Wahl 2006-04-18 21:26:51 UTC
Created attachment 127952 [details]
Fix for the buffer overflow

Comment 2 Neil Horman 2006-04-19 13:22:00 UTC
Created attachment 127983 [details]
alternate patch to fix buffer overflow

Thank you for the report, and it appears I'll need to take this upstream as
well.  However, given that it appears that the max term size truly is 512
bytes, I don't think just extending the size of the term array is the proper
solution, especially given that it implies that LINEMAX and TERMMAX always need
to be the same size.  This alternate patch should fix up the problem.  Could
you please test it and report back results?  Thanks

Comment 3 Ronald Wahl 2006-04-19 17:13:17 UTC
Created attachment 127994 [details]
Revised Patch

Well, your patch was incorrect. strcpy() has only 2 parameters. I guess you
mean
strncpy(). This function does not ensure that the destination is 0-terminated
if
the source is longer than the destination. You need to add a 0-byte manually
for
this case. See my new patch which works too.

Comment 4 Neil Horman 2006-04-19 17:37:56 UTC
yep, that was my intent.  Thanks.  I'll take care of this.

Comment 5 Bug Zapper 2008-04-04 02:40:26 UTC
Fedora apologizes that these issues have not been resolved yet. We're
sorry it's taken so long for your bug to be properly triaged and acted
on. We appreciate the time you took to report this issue and want to
make sure no important bugs slip through the cracks.

If you're currently running a version of Fedora Core between 1 and 6,
please note that Fedora no longer maintains these releases. We strongly
encourage you to upgrade to a current Fedora release. In order to
refocus our efforts as a project we are flagging all of the open bugs
for releases which are no longer maintained and closing them.
http://fedoraproject.org/wiki/LifeCycle/EOL

If this bug is still open against Fedora Core 1 through 6, thirty days
from now, it will be closed 'WONTFIX'. If you can reporduce this bug in
the latest Fedora version, please change to the respective version. If
you are unable to do this, please add a comment to this bug requesting
the change.

Thanks for your help, and we apologize again that we haven't handled
these issues to this point.

The process we are following is outlined here:
http://fedoraproject.org/wiki/BugZappers/F9CleanUp

We will be following the process here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping to ensure this
doesn't happen again.

And if you'd like to join the bug triage team to help make things
better, check out http://fedoraproject.org/wiki/BugZappers

Comment 6 Neil Horman 2008-04-04 13:50:39 UTC
hold on a sec, something just occured to me.  We use fgets in the surrounding
while loop in this code.  fgets takes a size parameter, and only reads size-1
bytes to the target buffer.  This implies that the strcpy you are fixing should
never overflow.  We shouldn't need to fix this in the way we're discussing.  As
such I don't feel compfortable incorporating this change.  I know you can't
release your repository, but is it possible for you to fabricate a repository
that can reproduce this error, so that I can look at it more closely?

Thanks!

Comment 7 Ronald Wahl 2008-04-08 22:30:03 UTC
Oh well, this was two years ago - the repository changed since then
considerably. Meanwhile I use cscope 15.6 which seems to work.

Comment 8 John Poelstra 2008-04-10 23:08:53 UTC
Neil-okay to close this re: comment #7 ?

Comment 9 Neil Horman 2008-04-11 00:00:02 UTC
yep, closing it.  Thanks!


Note You need to log in before you can comment on or make changes to this bug.