/hg/icedtea-web: Changed usage of sprintf to snprintf. Made some...
Deepak Bhole
dbhole at redhat.com
Tue May 22 11:39:32 PDT 2012
* adomurad at icedtea.classpath.org <adomurad at icedtea.classpath.org> [2012-05-22 14:30]:
> changeset c367faabd08e in /hg/icedtea-web
> details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=c367faabd08e
> author: Adam Domurad <adomurad at redhat.com>
> date: Tue May 22 14:19:50 2012 -0400
>
> Changed usage of sprintf to snprintf. Made some small malloc'd buffers on the stack.
> These changes are unlikely to change functionality, for if the buffer is too small to write to, something has already gone wrong. However, they are good as an additional safety guarantee, preventing memory from corruption in the case that something goes wrong.
Hi Adam,
Can you please wrap all comments on 80 for future commits? Makes it a
lot easier to read :)
Thanks!
Deepak
>
>
> diffstat:
>
> ChangeLog | 12 +++++++
> plugin/icedteanp/IcedTeaNPPlugin.cc | 6 +-
> plugin/icedteanp/IcedTeaPluginUtils.cc | 54 +++++++++++++++++----------------
> 3 files changed, 43 insertions(+), 29 deletions(-)
>
> diffs (169 lines):
>
> diff -r 7bbba99d25cc -r c367faabd08e ChangeLog
> --- a/ChangeLog Tue May 22 14:21:10 2012 +0200
> +++ b/ChangeLog Tue May 22 14:19:50 2012 -0400
> @@ -1,3 +1,15 @@
> +2012-05-22 Adam Domurad <adomurad at redhat.com>
> +
> + Changed allocation of small, fixed-size buffers to stack-based
> + allocations. Changed occurences of sprintf to the safer function
> + snprintf, added buffer information. While unlikely to change
> + functionality, snprintf adds an extra check to prevent buffer
> + overflows.
> + * plugin/icedteanp/IcedTeaNPPlugin.cc: Allocation of small buffers
> + using malloc changed to stack allocation & changed sprintf calls to
> + buffer-size aware snprintf calls.
> + * plugin/icedteanp/IcedTeaPluginUtils.cc: Same as above.
> +
> 2012-05-22 Jiri Vanek <jvanek at redhat.com>
>
> * tests/jnlp_tests/signed/ReadPropertiesSigned/testcases/ReadPropertiesSignedTest.java:
> diff -r 7bbba99d25cc -r c367faabd08e plugin/icedteanp/IcedTeaNPPlugin.cc
> --- a/plugin/icedteanp/IcedTeaNPPlugin.cc Tue May 22 14:21:10 2012 +0200
> +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc Tue May 22 14:19:50 2012 -0400
> @@ -1227,9 +1227,9 @@
> {
>
> // clear the "instance X status" parts
> - sprintf(parts[0], "");
> - sprintf(parts[1], "");
> - sprintf(parts[2], "");
> + snprintf(parts[0], sizeof(""), "");
> + snprintf(parts[1], sizeof(""), "");
> + snprintf(parts[2], sizeof(""), "");
>
> // join the rest
> gchar* status_message = g_strjoinv(" ", parts);
> diff -r 7bbba99d25cc -r c367faabd08e plugin/icedteanp/IcedTeaPluginUtils.cc
> --- a/plugin/icedteanp/IcedTeaPluginUtils.cc Tue May 22 14:21:10 2012 +0200
> +++ b/plugin/icedteanp/IcedTeaPluginUtils.cc Tue May 22 14:19:50 2012 -0400
> @@ -147,21 +147,20 @@
> IcedTeaPluginUtilities::JSIDToString(void* id, std::string* result)
> {
>
> - char* id_str = (char*) malloc(sizeof(char)*20); // max = long long = 8446744073709551615 == 19 chars
> + char id_str[20]; // max = long long = 8446744073709551615 == 19 chars
>
> if (sizeof(void*) == sizeof(long long))
> {
> - sprintf(id_str, "%llu", id);
> + snprintf(id_str, sizeof(id_str), "%llu", id);
> }
> else
> {
> - sprintf(id_str, "%lu", id); // else use long
> + snprintf(id_str, sizeof(id_str), "%lu", id); // else use long
> }
>
> result->append(id_str);
>
> PLUGIN_DEBUG("Converting pointer %p to %s\n", id, id_str);
> - free(id_str);
> }
>
> /**
> @@ -258,11 +257,9 @@
> IcedTeaPluginUtilities::itoa(int i, std::string* result)
> {
> // largest possible integer is 10 digits long
> - char* int_str = (char*) malloc(sizeof(char)*11);
> - sprintf(int_str, "%d", i);
> + char int_str[11];
> + snprintf(int_str, sizeof(int_str), "%d", i);
> result->append(int_str);
> -
> - free(int_str);
> }
>
> /**
> @@ -372,18 +369,17 @@
> ostream << length;
>
> // UTF-8 characters are 4-bytes max + space + '\0'
> - char* hex_value = (char*) malloc(sizeof(char)*10);
> + char hex_value[10];
>
> for (int i = 0; i < str->length(); i++)
> {
> - sprintf(hex_value, " %hx", str->at(i));
> + snprintf(hex_value, sizeof(hex_value)," %hx", str->at(i));
> ostream << hex_value;
> }
>
> utf_str->clear();
> *utf_str = ostream.str();
>
> - free(hex_value);
> PLUGIN_DEBUG("Converted %s to UTF-8 string %s\n", str->c_str(), utf_str->c_str());
> }
>
> @@ -683,49 +679,55 @@
> void
> IcedTeaPluginUtilities::NPVariantToString(NPVariant variant, std::string* result)
> {
> - char* str = (char*) malloc(sizeof(char)*32); // enough for everything except string
> + char str[32]; // enough for everything except string
> + char* largestr = NULL;
>
> if (NPVARIANT_IS_VOID(variant))
> {
> - sprintf(str, "%p", variant);
> + snprintf(str, sizeof(str), "%p", variant);
> }
> else if (NPVARIANT_IS_NULL(variant))
> {
> - sprintf(str, "NULL");
> + snprintf(str, sizeof(str), "NULL");
> }
> else if (NPVARIANT_IS_BOOLEAN(variant))
> {
> if (NPVARIANT_TO_BOOLEAN(variant))
> - sprintf(str, "true");
> + snprintf(str, sizeof(str), "true");
> else
> - sprintf(str, "false");
> + snprintf(str, sizeof(str), "false");
> }
> else if (NPVARIANT_IS_INT32(variant))
> {
> - sprintf(str, "%d", NPVARIANT_TO_INT32(variant));
> + snprintf(str, sizeof(str), "%d", NPVARIANT_TO_INT32(variant));
> }
> else if (NPVARIANT_IS_DOUBLE(variant))
> {
> - sprintf(str, "%f", NPVARIANT_TO_DOUBLE(variant));;
> + snprintf(str, sizeof(str), "%f", NPVARIANT_TO_DOUBLE(variant));;
> }
> else if (NPVARIANT_IS_STRING(variant))
> {
> free(str);
> #if MOZILLA_VERSION_COLLAPSED < 1090200
> - str = (char*) malloc(sizeof(char)*NPVARIANT_TO_STRING(variant).utf8length);
> - sprintf(str, "%s", NPVARIANT_TO_STRING(variant).utf8characters);
> + size_t buffersize = sizeof(char)*NPVARIANT_TO_STRING(variant).utf8length;
> + largestr = (char*) malloc(buffersize);
> + snprintf(str, buffersize, "%s", NPVARIANT_TO_STRING(variant).utf8characters);
> #else
> - str = (char*) malloc(sizeof(char)*NPVARIANT_TO_STRING(variant).UTF8Length);
> - sprintf(str, "%s", NPVARIANT_TO_STRING(variant).UTF8Characters);
> + size_t buffersize = sizeof(char)*NPVARIANT_TO_STRING(variant).UTF8Length;
> + largestr = (char*) malloc(buffersize);
> + snprintf(str, buffersize, "%s", NPVARIANT_TO_STRING(variant).UTF8Characters);
> #endif
> }
> else
> {
> - sprintf(str, "[Object %p]", variant);
> + snprintf(str, sizeof(str), "[Object %p]", variant);
> }
> -
> - result->append(str);
> - free(str);
> + if (largestr != NULL){
> + result->append(largestr);
> + free(largestr);
> + } else {
> + result->append(str);
> + }
> }
>
> bool
More information about the distro-pkg-dev
mailing list