Bug 151485

Summary: mDNSResponder-107.5-3: 3 * array subscript out of range
Product: [openSUSE] SUSE LINUX 10.0 Reporter: David Binderman <dcb314>
Component: BasesystemAssignee: Adrian Schröter <adrian.schroeter>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Minor    
Priority: P3 - Medium    
Version: unspecified   
Target Milestone: ---   
Hardware: All   
OS: SuSE Linux 10.1   
Whiteboard:
Found By: Other Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description David Binderman 2006-02-16 13:36:41 UTC
I just tried to compile package mDNSResponder-107.5-3 with the Intel C 
compiler.

It said

1.

../mDNSCore/DNSCommon.c(1401): warning #175: subscript out of range

The source code is

        while (nread < pktRDLen && (mDNSu8 *)opt < rr->rdata->u.data + MaximumRDSize - sizeof(rdataOpt))

but in the file ../BUILD/mDNSResponder-107.5/mDNSCore/mDNSEmbeddedAPI.h is
the line

	mDNSu8      data[StandardAuthRDSize];

and

#define StandardAuthRDSize 264
#define MaximumRDSize 8192

I compute sizeof( rdataOpt) to be about 78 bytes.

So it seems the program is trying to index (8192 - 78) == 8114 into an array
of size 264.

Suggest code rework.

2.

../mDNSCore/uDNS.c(3580): warning #175: subscript out of range

The source code is

	if (msg && msg->h.flags.b[2] >> 4 && msg->h.flags.b[2] >> 4 != kDNSFlag1_RC_NXDomain)

In file ../BUILD/mDNSResponder-107.5/mDNSCore/mDNSEmbeddedAPI.h, we see

typedef packedunion { mDNSu8 b[ 2]; mDNSu16 NotAnInteger; } mDNSOpaque16;

so msg->h.flags.b[2] does not exist. Suggest code rework.


3.

../mDNSCore/uDNS.c(3583): warning #175: subscript out of range

Duplicate.
Comment 1 Adrian Schröter 2006-02-16 14:10:15 UTC
Sorry, but don't support the Intel compiler. Please report this direct to Apple.
Comment 2 David Binderman 2006-05-20 07:40:53 UTC
(In reply to comment #1)
> Sorry, but don't support the Intel compiler. 

I am not asking you to support the Intel compiler.

The code is broken using any compiler, even using the GNU compiler. 

In fact, the code is still broken in the version that
ships with Suse Linux 10.1


Comment 3 Adrian Schröter 2006-11-14 14:02:16 UTC
okay, I don't understand the message. in 1., it does not do any copy functionality, it just compares values. How can the compiler know, if the assumptions are wrong ?

If you why, please send me a patch and explain it to me. For me, it looks bogus atm.
Comment 4 David Binderman 2006-11-14 17:12:49 UTC
(In reply to comment #3)
>How can the compiler know, if the assumptions are wrong ?

Perhaps you are not familiar with C.

The expression

rr->rdata->u.data + MaximumRDSize - sizeof(rdataOpt)

because u.data is an array, indexes into the array. Like

rr->rdata->u.data[ MaximumRDSize - sizeof(rdataOpt)]

This is a C FAQ.

The value of the array index is what the compiler is complaining about.

I tried to explain in my original bug report that array
indexing is occuring, and the array index is wrong. 

Detectable at compile time.

The second case is simpler. Array limits are exclusive in C,
which means an array declared to be b[ 2] has two elements,
b[ 0] and b[ 1]. b[ 2] does not exist and so is not accessable.

Hopefully that explains it. If you need any further help,
please get in touch.


Comment 5 Adrian Schröter 2006-11-14 17:43:14 UTC
I do understand that, but this action itself is not invalid, there happens no read/write action outside of u.data[] area in this statement. In the while() statement it just a comparisation of two pointers, even though the values might be bogus, it will not crash here. There is just one memcopy later where this memory gets accessed, it looks valid from first look (but the code is too complicate for me spot a problem within 10 seconds).

(I have to admit that I am not really keen on working in mDNSResponder, because we will most likely kick it just after 10.2 :/ )
Comment 6 David Binderman 2006-11-14 19:16:52 UTC
(In reply to comment #5)
> this action itself is not invalid, 

Wrong. The code is undefined. Anything could happen.

>there happens no read/write action outside of u.data[] area in this statement. 

True, but even computing the address is undefined. 
Any computation could result.

>In the while() statement it just a comparisation of two pointers, even though >the values might be bogus, it will not crash here. 

If you think it is reasonable to compare values that you know
to be at best bogus, then that is entirely your own choice.

> (I have to admit that I am not really keen on working in mDNSResponder, 
> because we will most likely kick it just after 10.2 :/ )

How keen you are is, at best, a side issue.

The code is clearly wrong, undefined and bogus and it will be 
shipping shortly as part of Suse Linux 10.2 GoldMaster.

There exist a few weeks to possibly implement some better code.

Comment 7 Stefan Dirsch 2007-01-16 22:32:48 UTC
Feel free to close as duplicate of Bug #235591.
Comment 8 Adrian Schröter 2007-06-11 08:12:02 UTC
mDNSResponder has been dropped meanwhile