Hi!
Replacing sprintf with snprintf is an improvement, but error handling is
still missing in the code below. snprintf() returns -1 in case of an
error, and it can return a value larger than or equal to
SA_MAX_NAME_LENGTH in case the concatenation below would have overflowed
the buffer. In these cases, SaNameT.length would get a value that is
0xffff or >= SA_MAX_NAME_LENGTH, respectively.
is_config_valid() is a validation function that seems to return 1 when
successful and 0 otherwise. So if the above case happens, I would guess
it is a good idea to return 0.
regards,
Anders Widell
2014-01-03 06:51, Gary Lee skrev:
> osaf/services/saf/amf/amfd/compcstype.cc | 5 +++--
> osaf/services/saf/amf/amfd/hlt.cc | 2 +-
> osaf/services/saf/amf/amfd/imm.cc | 9 ++++++---
> 3 files changed, 10 insertions(+), 6 deletions(-)
>
>
> * Calling risky function (SECURE_CODING)
>
> replace calls to sprintf with snprintf
>
> diff --git a/osaf/services/saf/amf/amfd/compcstype.cc
> b/osaf/services/saf/amf/amfd/compcstype.cc
> --- a/osaf/services/saf/amf/amfd/compcstype.cc
> +++ b/osaf/services/saf/amf/amfd/compcstype.cc
> @@ -204,7 +204,7 @@
> p = strchr(p, ',');
> *p = '\0';
>
> - ctcstype_name.length = sprintf((char*)ctcstype_name.value, "%s,%s",
> cstype_name, comptype_name->value);
> + ctcstype_name.length = snprintf((char*)ctcstype_name.value,
> SA_MAX_NAME_LENGTH, "%s,%s", cstype_name, comptype_name->value);
>
> if (avd_ctcstype_get(&ctcstype_name) == NULL) {
> if (opdata == NULL) {
> @@ -256,7 +256,8 @@
> p = strchr(cstype_name, ',') + 1;
> p = strchr(p, ',');
> *p = '\0';
> - ctcstype_name.length = sprintf((char*)ctcstype_name.value,
> + ctcstype_name.length = snprintf((char*)ctcstype_name.value,
> + SA_MAX_NAME_LENGTH,
> "%s,%s", cstype_name, comp->comp_type->name.value);
>
> ctcstype = avd_ctcstype_get(&ctcstype_name);
> diff --git a/osaf/services/saf/amf/amfd/hlt.cc
> b/osaf/services/saf/amf/amfd/hlt.cc
> --- a/osaf/services/saf/amf/amfd/hlt.cc
> +++ b/osaf/services/saf/amf/amfd/hlt.cc
> @@ -115,7 +115,7 @@
>
> comp_name = strstr((char *)opdata->objectName.value, "safComp");
> osafassert(comp_name);
> - comp_dn.length = sprintf((char *)comp_dn.value, "%s", comp_name);
> + comp_dn.length = snprintf((char *)comp_dn.value, SA_MAX_NAME_LENGTH,
> "%s", comp_name);
> comp = avd_comp_get(&comp_dn);
> osafassert(comp);
>
> diff --git a/osaf/services/saf/amf/amfd/imm.cc
> b/osaf/services/saf/amf/amfd/imm.cc
> --- a/osaf/services/saf/amf/amfd/imm.cc
> +++ b/osaf/services/saf/amf/amfd/imm.cc
> @@ -725,15 +725,18 @@
> if (attrValue->attrValueType == SA_IMM_ATTR_SASTRINGT) {
> SaStringT rdnVal = *((SaStringT
> *)attrValue->attrValues[0]);
> if ((parent_name != NULL) &&
> (parent_name->length > 0)) {
> - operation->objectName.length =
> sprintf((char *)operation->objectName.value,
> + operation->objectName.length =
> snprintf((char *)operation->objectName.value,
> + SA_MAX_NAME_LENGTH,
> "%s,%s", rdnVal,
> parent_name->value);
> } else {
> - operation->objectName.length =
> sprintf((char *)operation->objectName.value,
> + operation->objectName.length =
> snprintf((char *)operation->objectName.value,
> + SA_MAX_NAME_LENGTH,
> "%s", rdnVal);
> }
> } else {
> SaNameT *rdnVal = ((SaNameT
> *)attrValue->attrValues[0]);
> - operation->objectName.length = sprintf((char
> *)operation->objectName.value,
> + operation->objectName.length = snprintf((char
> *)operation->objectName.value,
> + SA_MAX_NAME_LENGTH,
> "%s,%s", rdnVal->value,
> parent_name->value);
> }
>
>
> ------------------------------------------------------------------------------
> Rapidly troubleshoot problems before they affect your business. Most IT
> organizations don't have a clear picture of how application performance
> affects their revenue. With AppDynamics, you get 100% visibility into your
> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
> http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
> _______________________________________________
> Opensaf-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>
------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
