Hi folks.

Yes, ares_set_servers_csv() returned what it should. It’s just the behavior 
afterwards which is not correct. I’ll submit PR for adding this check, thanks.

. .:|:.:|:. Francisco Sedano  | CCIE 14859, Tech Lead Software Engineering | 
CSG Enterprise Access and Services Group (EASG)

From: c-ares <c-ares-boun...@cool.haxx.se> on behalf of David Drysdale 
Reply-To: c-ares hacking <c-ares@cool.haxx.se>
Date: Sunday, 11 March 2018 at 08:29
To: c-ares hacking <c-ares@cool.haxx.se>
Subject: Re: Wrong handling on badly formatted strings passed to set_servers_csv

On Sat, Mar 10, 2018 at 1:03 PM, Brad House 
<b...@mainstreetsoftworks.com<mailto:b...@mainstreetsoftworks.com>> wrote:
Did ares_set_servers_csv() return ARES_EBADSTR as it should?

There's a test that implies that it will: 

It does appear when it does that, it leaves the channel in a "bad" state since 
existing servers are cleared before parse. I'm not sure of any other instances 
where a channel might have no servers, as even in the case where it can't 
determine one from configuration, it uses a localhost fallback.  It may make 
more sense to kill the ares__destroy_servers_state()  call at the beginning of 
set_servers_csv(), since ares_set_servers_ports() clears it which gets called 
after a successful parse.

I'd agree that there should probably be a sanity check in ares_send() to not 
try an invalid ares_malloc(), ARES_ESERVFAIL would be as good an error 
condition as any in this case.



On 3/9/18 7:07 PM, Francisco Sedano Crippa (fsedanoc) wrote:


I noticed today if you pass a string with spaces to set_servers_csv, like:

" ,"

It will take the first server as " " (note the space), it will notice 
it's not a valid IP and fail. So far so good.

However, nservers for the channel will stay set to -1, so when ares_send is 
called, this will be executed:

  query->server_info = ares_malloc(channel->nservers *

The negative value will be misinterpreted to a huge number since argument is 
size_t and we agree things smell really bad from here. In practice, such a mem 
allocation fails and we return ENOMEM (which is also misleading), but it's a 
very incorrect behaviour.

I was thinking on just adding a check at the beginning of ares_send() to exit 
if nservers is <= 0.

Do you guys agree with the approach? If that’s the case, which error do you 
suggest to return? No one really matches, I’d say ARES_ENOTFOUND, but that 
implies we tried to contact the server…


. .:|:.:|:. Francisco Sedano  | CCIE 14859, Tech Lead Software Engineering | 
CSG Enterprise Access and Services Group (EASG)

Reply via email to