RE: smsc_at crashes on non-numeric SMS destination

Supressing SMS delivery to short codes shouldn't be a feature hard-coded
into the code.
Why not consider black-list-regex/white-list-regex instead?

Andrija

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


On Tue, May 09, 2006 at 02:50:39PM +0200, Andrija Petrovic wrote:
> 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...
well, if you'd like to send messages to vas numbers... this was which i
wanted it not to do.
you can make the bill rather high in a verry short amount of time that way.


> 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.
you may be right at this point.

>
> 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...
well, thats exactly what i was asking...
>
> 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
>
>
>
>
>
>
>

--
Wilfried GĂśsgens
Newthinking Communications