RE: smsc_at crashes on non-numeric SMS destination

I'm not quite sure that your proposal will nicely fix the thing:

1. msg->sms.receiver should be allowed to be shorter than 6 digits, why
not...
2. as far as at2_format_address_field is concerned, digits may be hex

So, why not use the patch I provided? It's simple and safe... If
at2_format_address_field fails, it will return NULL, that will be catched
inside the caller function at2_pdu_encode, so it will return NULL as well,
so at2_send_one_message (currently) will generate the error(2, ...) message
and safely return.

Anyway, AFAIC, I'd let the at2_format_address_field perform the sanity
checks and (possibly) the very conversion, and not perfom any ad-hoc checks
in outer functions.

Now, about destroying the message, take a look at how you get to the
at2_send_one_message:
at2_send_messages removes the message from the queue and calls
at2_send_one_message.
It seems that no destroying of the received pointer should be done at all,
correct me if I'm wrong...

Andrija

-----Original Message-----
From: [email protected] [mailto:[email protected]]On
Behalf Of Wilfried Goesgens
Sent: Tuesday, May 09, 2006 2:21 PM
To: [email protected]
Subject: Re: smsc_at crashes on non-numeric SMS destination


On Tue, May 09, 2006 at 02:07:42PM +0200, Andrija Petrovic wrote:
> smsc_at does not check the digits of the address field in the function
> at2_format_address_field
> So, if the recipient's address contains a non-numeric string (e.g. 'Info
> Service'), and that's quite possible,
> the octstr assertion crashes the bearerbox during octstr_append_char.
>
> Added sanity check on digits before calling the octstr_append_char.
>
> cheers,
> Andrija


i've added the following to send_one_message to fix this:

	if ((octstr_len(msg->sms.receiver)<6)||
		(octstr_parse_double(&number, msg->sms.receiver,0)==-1))
		{/* ok. this message is faulty. no shortcodes or non number targets */
            error(0, "AT4[%s]: bad receiver %s. text %s dropping!",
				  octstr_get_cstr(privdata->name),
				  octstr_get_cstr(msg->sms.receiver),
				  octstr_get_cstr(msg->sms.msgdata));
			if( DLR_IS_ENABLED_DEVICE(msg->sms.dlr_mask))
				bb_smscconn_send_failed(privdata->conn, msg,
										SMSCCONN_FAILED_MALFORMED, octstr_create("MALFORMED"));
			return 0;

		}

is it correct, that the message is destroyed inside bb_smscconn_send_failed?
if i destroy it after that, i get trouble...


Wilfried GĂśsgens