Bug 133124

Summary: array
Product: [Fedora] Fedora Reporter: David Binderman <dcb314>
Component: starAssignee: Peter Vrabec <pvrabec>
Status: CLOSED WONTFIX QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: 3CC: schily
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2004-10-18 14:55:54 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description David Binderman 2004-09-21 19:28:39 UTC
Description of problem:

I just tried to compile package star-1.5a25-6 from
Redhat Fedora Core 3 Test 1.

The compiler said

1.

header.c(1271): warning #175: subscript out of range

The source code is

        if (ptb->dbuf.t_name[NAMSIZ] == '\0') {

Suggest

        if (ptb->dbuf.t_name[NAMSIZ - 1] == '\0') {

might be better code.

2.

header.c(1283): warning #175: subscript out of range

The source code is

                ptb->dbuf.t_name[NAMSIZ] = ' ';

This will never work in a month of Sundays. Try

                ptb->dbuf.t_name[ NAMSIZ - 1] = ' ';

3.

header.c(1372): warning #175: subscript out of range
header.c(1373): warning #175: subscript out of range
header.c(1374): warning #175: subscript out of range
header.c(1375): warning #175: subscript out of range
header.c(1376): warning #175: subscript out of range
header.c(1377): warning #175: subscript out of range

The source code is

        xname = ptb->dbuf.t_name[NAMSIZ];
        ptb->dbuf.t_name[NAMSIZ] = '\0';        /* allow 100 chars in
name */
        xlink = ptb->dbuf.t_linkname[NAMSIZ];
        ptb->dbuf.t_linkname[NAMSIZ] = '\0'; /* allow 100 chars in
linkname */
        xpfx = ptb->dbuf.t_prefix[PFXSIZ];
        ptb->dbuf.t_prefix[PFXSIZ] = '\0';      /* allow 155 chars in
prefix*/

Crikey, someone doesn't know much about exclusive upper limits on 
arrays in C.

4.

header.c(1385): warning #175: subscript out of range
header.c(1386): warning #175: subscript out of range
header.c(1387): warning #175: subscript out of range

The source code is

        ptb->dbuf.t_name[NAMSIZ] = xname;       /* restore remembered
value */
        ptb->dbuf.t_linkname[NAMSIZ] = xlink;   /* restore remembered
value */
        ptb->dbuf.t_prefix[PFXSIZ] = xpfx;      /* restore remembered
value */

More of the same.

5.

tartest.c(249): warning #175: subscript out of range

The source code is

            ptb->ustar_dbuf.t_name[100] == '\0') {

6.

tartest.c(255): warning #175: subscript out of range

Duplicate.



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


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 Peter Vrabec 2004-10-18 14:55:54 UTC
Try report to the upstream. I think it's a hack. Ugly, but it works.


Comment 2 David Binderman 2004-10-18 20:36:32 UTC
>Try report to the upstream.

Sorry, haven't got time to do this.

I had rather hoped a member of the redhat team might fix it.

Certainly, when I report similar bugs in other packages in redhat
fedora Core 3 test 3, my bug reports get fixed.

>I think it's a hack. Ugly, but it works.

I think it's broken code, which needs fixing.


Comment 3 Warren Togami 2004-10-27 10:51:45 UTC
> Sorry, haven't got time to do this.
> I had rather hoped a member of the redhat team might fix it.

This is a poor excuse.  If you truly care about this issue, then you
would put the effort into reporting it properly, or at least beginning
the discussion in the upstream project mailing list.

On the other hand, if you can demonstrate that this causes a crash or
security problem, then it quickly becomes a RH problem.



Comment 4 Alan Cox 2004-10-27 13:25:06 UTC
I've pointed Joerg at the bug report. He's a very active maintainer of
his packages so I'm sure he'll be glad to get the report.


Comment 5 Alan Cox 2004-10-27 13:48:06 UTC
Joerg says

"This is an extremely outdated version of star (more than a year).   
                       
The current release is 1.5a52. 
The code is 100% correct - ignore the compiler warnings."



Comment 6 James Antill 2004-10-27 16:25:09 UTC
> The code is 100% correct - ignore the compiler warnings.

 This is not true for the RHEL3 code, or the latest "alpha" code I can
download from ftp://ftp.berlios.de/pub/star ... the 1.5a52.

 The star.h file is...

#define NAMSIZ          100
[...]
struct star_header {
        char t_name[NAMSIZ];        /*   0 Dateiname                    */
[...]
typedef union hblock {
[...]
        struct star_header dbuf;
[...]
} TCB;

...the header.c file is...

        if (ptb->dbuf.t_name[NAMSIZ] == '\0') {
[...]
                ptb->dbuf.t_name[NAMSIZ] = ' ';
[...]
        xname = ptb->dbuf.t_name[NAMSIZ];
        ptb->dbuf.t_name[NAMSIZ] = '\0';        /* allow 100 chars in
name */

...even the comment shows that he knows it's wrong.
 ptb is a pointer to the TCB union, NAMSIZ is only ever defined to be
100 ... so he's not being "clever" there either.

 It's probably not exploitable.


Comment 7 David Binderman 2004-11-05 19:47:38 UTC
I can confirm the version of star in Fedora Core 3 Test 3 
still has these array index bugs.



Comment 8 David Binderman 2005-10-13 11:26:49 UTC
Alan Cox writes:
>I've pointed Joerg at the bug report. 

Thanks. I've tried emailing him, with no answer. Is his 
email address in the package out of date ?

>He's a very active maintainer of
>his packages so I'm sure he'll be glad to get the report.

Interesting, however this code is still broken a year later.

Version 1.5a68-1, so I suspect there have been quite a few versions
since 1.5a25-6.

Would the star package benefit from a movement into the extras
area ?

Comment 9 Jörg Schilling 2009-01-25 12:55:31 UTC
The original code is 100% correct.

With the proposed patch, the code would no longer be correct.

If you like to make a bug report please make a bug report
against the C-compiler as this compiler does not honor the
/* LINTED */ comment:

        /* LINTED */ 
        if (ptb->dbuf.t_name[NAMSIZ] == '\0') {