Ack. Thanks -Nagu
-----Original Message----- From: Nagendra Kumar Sent: 20 August 2013 16:58 To: Hans Feldt; Praveen Malviya Cc: [email protected] Subject: Re: [devel] [PATCH 1 of 1] amfnd: improve validation of received msgs [#310] We are testing the patch, early review comment inlined with [Nagu] Thanks -Nagu -----Original Message----- From: Hans Feldt [mailto:[email protected]] Sent: 09 August 2013 20:22 To: Praveen Malviya Cc: [email protected] Subject: [devel] [PATCH 1 of 1] amfnd: improve validation of received msgs [#310] osaf/services/saf/avsv/avnd/avnd_evt.c | 6 ++++-- osaf/services/saf/avsv/avnd/avnd_mds.c | 31 ++++++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 7 deletions(-) When adding new message types in context of #83 it was discovered that amfnd was lacking some validation of message content. With some bad input amfnd either asserted or crashed with seg fault which resulted in node reboot. This patch improves the situation by discarding unknown message types, adding checks for message len, discarding messages from unknown SVC IDs. diff --git a/osaf/services/saf/avsv/avnd/avnd_evt.c b/osaf/services/saf/avsv/avnd/avnd_evt.c --- a/osaf/services/saf/avsv/avnd/avnd_evt.c +++ b/osaf/services/saf/avsv/avnd/avnd_evt.c @@ -177,7 +177,9 @@ AVND_EVT *avnd_evt_create(AVND_CB *cb, break; default: - osafassert(0); + free(evt); + evt = NULL; + break; } done: @@ -295,7 +297,7 @@ void avnd_evt_destroy(AVND_EVT *evt) case AVND_EVT_PID_EXIT: break; default: - osafassert(0); + break; [Nagu]: Please print message type here. } /* free the avnd event */ diff --git a/osaf/services/saf/avsv/avnd/avnd_mds.c b/osaf/services/saf/avsv/avnd/avnd_mds.c --- a/osaf/services/saf/avsv/avnd/avnd_mds.c +++ b/osaf/services/saf/avsv/avnd/avnd_mds.c @@ -446,6 +446,19 @@ uint32_t avnd_mds_rcv(AVND_CB *cb, MDS_C msg.type = AVND_MSG_AVA; msg.info.ava = (AVSV_NDA_AVA_MSG *)rcv_info->i_msg; + + if (msg.info.ava->type != AVSV_AVA_API_MSG) { + LOG_NO("%s: unknown message type (%u)", __FUNCTION__, + msg.info.ava->type); + goto done; + } + + if ((msg.info.ava->info.api_info.type == 0) || + (msg.info.ava->info.api_info.type >= AVSV_AMF_API_MAX)) { + LOG_NO("%s: unknown API type (%u)", __FUNCTION__, + msg.info.ava->info.api_info.type); + goto done; + } break; [Nagu]: Return(rc) is still success, I think it should be failure. case NCSMDS_SVC_ID_AVND: @@ -1016,7 +1029,8 @@ uint32_t avnd_mds_dec(AVND_CB *cb, MDS_C break; default: - osafassert(0); + LOG_NO("%s: unknown 'from' SVC ID %u", __FUNCTION__, dec_info->i_fr_svc_id); + rc = NCSCC_RC_FAILURE; break; } @@ -1092,7 +1106,8 @@ uint32_t avnd_mds_flat_dec(AVND_CB *cb, break; default: - osafassert(0); + LOG_NO("%s: unknown 'from' SVC ID %u", __FUNCTION__, dec_info->i_fr_svc_id); + rc = NCSCC_RC_FAILURE; break; } @@ -1114,12 +1129,18 @@ uint32_t avnd_mds_flat_dec(AVND_CB *cb, uint32_t avnd_mds_flat_ava_dec(AVND_CB *cb, MDS_CALLBACK_DEC_INFO *dec_info) { AVSV_NDA_AVA_MSG *ava_msg = 0; - uint32_t rc = NCSCC_RC_SUCCESS; + uint32_t rc = NCSCC_RC_FAILURE; - /* alloc memory for ava-msg */ + /* AMF library always sends a fixed size message, verify that size so that + * the decode routine does not read over the edge */ + if (dec_info->io_uba->max != sizeof(AVSV_NDA_AVA_MSG)) { + LOG_NO("%s: wrong number of bytes to decode (%u vs %lu)", + __FUNCTION__, dec_info->io_uba->max, sizeof(AVSV_NDA_AVA_MSG)); + goto err; + } + ava_msg = calloc(1, sizeof(AVSV_NDA_AVA_MSG)); if (!ava_msg) { - rc = NCSCC_RC_FAILURE; goto err; } ------------------------------------------------------------------------------ Get 100% visibility into Java/.NET code with AppDynamics Lite! It's a free troubleshooting tool designed for production. Get down to code-level detail for bottlenecks, with <2% overhead. Download for free and get started troubleshooting in minutes. http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Introducing Performance Central, a new site from SourceForge and AppDynamics. Performance Central is your source for news, insights, analysis and resources for efficient Application Performance Management. Visit us today! http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more! Discover the easy way to master current and previous Microsoft technologies and advance your career. Get an incredible 1,500+ hours of step-by-step tutorial videos with LearnDevNow. Subscribe today and save! http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
