Feedback on potential fix for issue while advertising Feature List

Feedback on potential fix for issue while advertising Feature List

[Bart Westgeest]

Hi folks,

I've tracked down the root cause of an issue, and am looking for 
input/feedback on what the acceptable/preferred fix is.

The code of interest is in sdp_set_supp_feat. The bug is exposed by 
registering a feature list (SDP_ATTR_SUPPORTED_FEATURES_LIST) of a 
length greater than 256. The attribute sequence type in one of the 
resulting attribute sequences is hard-coded to be SDP_SEQ8. When the 
list grows beyond 256 bytes, the size descriptor type is not corrected 
to SDP_SEQ16.

Henceforth, the indicated size of this attribute sequence in an SDP 
response PDU is whatever the lower byte of the feature list size is. 
However, the complete list data is written in the SDP response, hence 
forming a corrupted PDU.

This bug can easily be reproduced by registering a number of Health 
Applications (including a description string) such that the size of the 
advertised feature list attribute is greater than 256 bytes. 
Subsequently querying for this SDP record with sdptool, will result in 
indicated errors about unknown data due to parsing errors.

The quick and dirty fix is to change the return statement in 
sdp_seq_alloc (lib/sdp.c:563) from:
	return sdp_data_alloc(SDP_SEQ8, seq);
to
	return sdp_data_alloc(SDP_SEQ16, seq);

However, it really looks like the data type should be determined 
dynamically, or perhaps (a more involved fix,) instead of using nested 
data sequences for the outer data type, perhaps it should be changed to 
a list i.e. to use sdp_list_t instead sdp_data_t for the feature list.

In addition, this problem (hard coded sequence length for a variable 
length list) seems be repeated at various places throughout the code, so 
this problem does not appear to be isolated to sdp_set_supp_feat/HDP. 
For example sdp_set_lang_attr, sdp_set_uuidseq_attr, and 
sdp_set_profile_descs all appear to form corrupted PDU if their 
specified list grows beyond 256.

I appreciate any input from anybody more familiar with the SDP code on 
what the accepted/preferred approach is to get this issue fixed.

Thanks,
Bart

Re: Feedback on potential fix for issue while advertising Feature List

[Bart Westgeest]

 > I appreciate any input from anybody more familiar with the SDP
 > code on what the accepted/preferred approach is to get this
 > issue fixed.

Anybody? Again, I've already root caused the problem and have a fix.

I am Just looking for input to see if what I have is acceptable, or if 
something else is preferred.

Re: Feedback on potential fix for issue while advertising Feature List

[Johan Hedberg]

Hi Bart,

On Fri, Nov 09, 2012, Bart Westgeest wrote:
> > I appreciate any input from anybody more familiar with the SDP
> > code on what the accepted/preferred approach is to get this
> > issue fixed.
> 
> Anybody? Again, I've already root caused the problem and have a fix.
> 
> I am Just looking for input to see if what I have is acceptable, or
> if something else is preferred.

Have a bit more patience, sometimes it can take a few days to get
feedback. I think the option of looking at the length and using the old
type for < 256 and a 16-bit type for greater lengths would be the
safest. This way old code will still produce the same result for < 256
lengths.

Johan

Re: Feedback on potential fix for issue while advertising Feature List

[Bart Westgeest]

> I think the option of looking at the length and using the old
> type for < 256 and a 16-bit type for greater lengths would be the
> safest.

Thanks for the feedback. I'll start working on a patch.