AH! I see. Sorry for my mis-interpretation.
That definitely is something that should be audited. I see that
OCResource.cpp uses MAX_ADDR_STR_SIZE (note the lack of _CA), which
won't match unless a similar #ifdef is happening too.
What you point out is definitely a pretty nasty gotcha that needs
looking into closely.
On Thu, 2015-11-05 at 18:24 +0000, Light, John J wrote:
> Erich,
>
> You describe a problem that was fixed. I was referring to something else.
>
> OCDevAddr and CAEndpoint_t are both defined thusly:
>
> typedef struct
> {
> CATransportAdapter_t adapter; // adapter type
> CATransportFlags_t flags; // transport modifiers
> uint16_t port; // for IP
> char addr[MAX_ADDR_STR_SIZE_CA]; // address for all
> uint32_t interface; // usually zero for default
> interface
> #if defined (ROUTING_GATEWAY) || defined (ROUTING_EP)
> char routeData[MAX_ADDR_STR_SIZE_CA]; /**<
> GatewayId:ClientId of
>
> destination. **/
> #endif
> } CAEndpoint_t;
>
> Notice that both addr and routeData are defined as MAX_ADDR_STR_SIZE_CA.
> Since 40 is not long enough for the routeData, the author set
> MAX_ADDR_STR_SIZE_CA as follows:
>
> #ifdef RA_ADAPTER
> #define MAX_ADDR_STR_SIZE_CA (256)
> #else
> #define MAX_ADDR_STR_SIZE_CA (40)
> #endif
>
> So when RA_ADAPTER is defined, the host length will be wrong, possibly
> causing future problems. Without examining the code, I'm concerned that
> MAX_ADDR_STR_SIZE_CA is used elsewhere as a general string length.
>
> John
>
>
> -----Original Message-----
> From: Keane, Erich
> Sent: Thursday, November 05, 2015 9:18 AM
> To: Light, John J
> Cc: junghyun.oh at samsung.com; jihun.ha at samsung.com; iotivity-dev at
> lists.iotivity.org
> Subject: Re: [dev] Pre-defined length for the Host Info. is not long enough
>
> This was actually fixed this one a while ago. I believe there to be no large
> problem.
>
> What ended up happening is we were testing the length of the host address in
> C++ BEFORE pulling the address apart. I changed it to validate after (right
> before the copy), and it ended up fixing the problem, since (like you said),
> the actual values ARE less than 40 bytes long.
>
> the "coaps://[fe...]:36238" was NEVER put into a buffer defined by
> MAX_ADDR_STR_SIZE, it was taken in the std::string 'host' parameter to
> OCResource's ctor.
>
> I believe the cause was at one point we fixed a different issue by making the
> .host() call return the fully qualified string, rather than just the address.
> The result was the previous assumption (that the host was a valid,
> address-only component that could be immediately copied in) was made false.
>
> You can see here:
> https://gerrit.iotivity.org/gerrit/#/c/2481/6/resource/src/OCResource.cpp
> exactly what I mean.
>
> On Thu, 2015-11-05 at 16:00 +0000, Light, John J wrote:
> > Jay,
> >
> >
> >
> > Please provide more information about your issue. I originally wrote
> > the data structures you are talking about, and I am concerned there
> > might be a larger problem now.
> >
> >
> >
> > The host address field should only contain the address string itself.
> > The longest of these is a full IPv6 address:
> > ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff, which is 39 bytes long, plus
> > one for a terminating null. What other information are you seeing in
> > the string?
> >
> > You report seeing ?coaps://[fe80::a00:27ff:feaf:f911]:36238?, but that
> > should never be put in a buffer defined by MAX_ADDR_STR_SIZE. Can you
> > say where that happened?
> >
> > The string MAX_ADDR_STR_SIZE should only be used or the host field of
> > an OCDevAddr or CAEndpoint_t. I see it is now used for other fields,
> > and with other sizes than 40.
> >
> >
> >
> > This misuse will lead to larger problems in future. It is an addition
> > to the considerable technical debt we have on this project.
> >
> >
> >
> > John Light
> >
> > Intel OTC OIC Development
> >
> >
> >
> >
> >
> >
> >
> > From: iotivity-dev-bounces at lists.iotivity.org
> > [mailto:iotivity-dev-bounces at lists.iotivity.org] On Behalf Of ???
> > Sent: Wednesday, November 04, 2015 5:39 PM
> > To: ???; 'iotivity'
> > Subject: Re: [dev] Pre-defined length for the Host Info. is not long
> > enough
> >
> >
> >
> >
> > Hi. Jay,
> >
> >
> >
> > I also encountered the situation you described below, but I found that
> > this problem has been already resolved in the master branch with a
> > patchset, https://gerrit.iotivity.org/gerrit/#/c/2481/. Please look
> > into it.
> >
> >
> >
> > However, I don't know why this patch had not been merged into
> > 1.0.0-dev branch. In the 1.0.0-dev branch which is 1.0 release branch,
> > there is still the above problem. May be someone missed merging the
> > patchset into the branch.
> >
> >
> >
> > Thank you.
> >
> >
> >
> > BR, Jihun Ha
> >
> >
> >
> > ------- Original Message -------
> >
> > Sender : ???<junghyun.oh at samsung.com> S5(??)/??/IoT Lab(S/W?? )/????
> >
> > Date : 2015-11-04 17:24 (GMT+09:00)
> >
> > Title : [dev] Pre-defined length for the Host Info. is not long enough
> >
> >
> >
> > Hi all,
> >
> >
> >
> > Currently, I?m trying to port the ?Secure? version of the IoTivity
> > into an embedded device.
> >
> > While I was tried to create a resource object of a remote resource
> > from the Resource Client
> >
> > using an API ?constructResourceObject?, I found that the Stack doesn?t
> > allow the host infor.
> >
> > of which length exceeds 40 bytes.
> >
> > Actually the target Resource server was supporting IPv6 and using
> > secure version of IoTivity,
> >
> > the length of the host infor exceeds 40 bytes.
> >
> > Here is the error log & the actual string of the host infor of the
> > Resource Server..
> >
> > ? Error Log
> >
> > terminate called after throwing an instance of 'std::length_error'
> > what(): host address is too long.
> >
> >
> >
> > ? Host infor.
> >
> > coaps://[fe80::a00:27ff:feaf:f911]:36238
> >
> >
> >
> > As you can notice the length of the host infor. is 41 bytes.(including
> > ?\0? at the end).
> >
> > Since the variables defined for the MAX length of the host infor. are
> > exists both in the RI layer & CA layer
> >
> > & used by lots of locations inside the both layers, I think it will be
> > better if the contributors of the both layers
> >
> > take a look at this and fix if necessary.
> >
> > (CA : cacommon.h MAX_ADDR_STR_SIZE_CA 40 , RI : octypes.h
> > MAX_ADDR_STR_SIZE 40)
> >
> >
> >
> > I?ve created an issue on Jira
> > (https://jira.iotivity.org/browse/IOT-824)
> >
> > I really will be appreciated if someone resolve this issue. J
> >
> >
> >
> > Thank you.
> >
> > Jay.
> >
> >
> >
> >
> >
> > ?????.?????.
> >
> >
> >
> > Best Regards,
> >
> >
> >
> > Jihun Ha (???/???, Ph.D.)
> >
> > IoT, IoTivity, OIC| IoT Solution Lab
> >
> > Software R&D Center | Samsung Electronics Co., Ltd
> >
> > Mobile +82 10 2533 7947
> >
> > jihun.ha at samsung.com | jhha85 at gmail.com
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > _______________________________________________
> > iotivity-dev mailing list
> > iotivity-dev at lists.iotivity.org
> > https://lists.iotivity.org/mailman/listinfo/iotivity-dev
>
