#100 (1.6.15) uninitialized index in httpparser.c

[zephyrus]

upnp/src/genlib/net/http/httpparser.c
In function parser_parse_requestline(), a local variable index is not initialized properly
when it is referenced

near 1313 th line:
/* HTTP version equals to 1.0 should fail for MSEARCH as required by the
* UPnP certification tool */
hmsg->major_version < 0 || ( ( hmsg->major_version == 1 )
&& ( hmsg->minor_version < 1 )
&& ( Http_Method_Table[index].id == HTTPMETHOD_MSEARCH ) ) ) {
parser->http_error_code = HTTP_HTTP_VERSION_NOT_SUPPORTED;

Both gcc4 and Oracle Solaris Suite compiler suggested that index may not be initialized when
it was referenced here. So I got curious and initialized index to (-1) and put an assert(index >=0).
I had to compile the library with --enable-debug to enable assert() [otherwise assert() is NOP].
The assert triggered abort. index is not initialized properly.

I am only reporting a problem here, but not sure how to solve it.

[zephyrus]

How to trigger the error. (--enable-debug is required.)

[Fabrice Fontaine]

Hi,

Thanks for this report, the bug will be fixed in next upnp release as there was already a tracker ID for it (3494865). So, I will put this entry as a duplicate.

Best Regards,

Fabrice

[Marco Virgulti]

Hello to all..I'm new here but i'm using libupnp for a few months. I've encountered the same bug in the index variable in httpparser.c
I've compiled all with MSVC2008 and i got the error when trying to connect with the http request string:

GET <request> HTTP/1.0

I found that some old control points use this kind of request ( my samsung ledtv for example).
In this case, the first parsing instruction (near line 1250):

status =
match( &parser->scanner, "%s\t%S%w%c", &method_str, &url_str );

returns != PARSE_OK, so it goes on but at the second parsing instruction ( near line 1289):

status = match( &parser->scanner,
"%s\t%S\t%ihttp%w/%w%L%c", &method_str, &url_str,
&version_str );
it returns == PARSE_OK but it doesn't set index matching the right HTTP method, so in the next if statement (only with HTTP 1.0 version) it will fail with segfault (msvc doesn't set the vars). So i moved the code after that if before and all is solved. that's my new code:

index =
map_str_to_int( method_str.buf, method_str.length,
Http_Method_Table, NUM_HTTP_METHODS, TRUE );
if( index < 0 ) {
/* error; method not found */
parser->http_error_code = HTTP_NOT_IMPLEMENTED;
return PARSE_FAILURE;
}

if( num_scanned != 2 ||
/* HTTP version equals to 1.0 should fail for MSEARCH as required by the
* UPnP certification tool */
hmsg->major_version < 0 || ( ( hmsg->major_version == 1 )
&& ( hmsg->minor_version < 1 )
&& ( Http_Method_Table[index].id == HTTPMETHOD_MSEARCH ) ) ) {
parser->http_error_code = HTTP_HTTP_VERSION_NOT_SUPPORTED;
/* error; bad http version */
return PARSE_FAILURE;
}

hope this helps...

[Marco Virgulti]

excuse for my bad english!

[zephyrus]

Dear Fabrice and mvirg83,

Thank you for the follow up.

I looked at 3494865) and moved
the checking code that set index slightly earlier in the code (and that is essentailly
the fix suggested by mvirg83).

The code seems to work, and I noticed a different problem, a memory access beyond
allocated area (by malloc). and I will report it in a new entry.

Thank you for follow-up again.