Request for approval: Bug 100057 (original) (raw)

Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties

Andrew Haley aph at redhat.com
Fri May 22 16:44:20 UTC 2009

Previous message: Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties
Next message: Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Alan Bateman wrote:

Andrew Haley wrote:

https://bugs.openjdk.java.net/showbug.cgi?id=100057

GetJavaProperties has a stack-allocated fixed size buffer for holding a copy of a string returned by setlocale(3). However, there is no guarantee that the string will fit into this buffer. This one is probably due to Solaris code being reused for Linux. The patch has been in IcedTea for a long while. OK to push, OpenJDK 7 and 6?

I can't say I know this code very well but I see that the full-locale name can also be copied into temp when the locale is an alias. This makes me wonder if temp might need to be realloc'ed there? Also, I wonder if the return from malloc should be checked.

I've created a corresponding sunbug for this: 6844255: Potential stack corruption in GetJavaProperties

I've reworked the patch.

I tried to refactor the code to make it cleaner and easier to follow, but I ended up touching so many places that it bacame a major change, and there's no way I could possibly test it all, especially the special cases for Solaris.

So, I backed off and produced this, the minimum patch. encoding_variant and temp are malloc()d to the size that they need, and temp is realloc()d, as you suggest.

Andrew.

diff -r 238591c80bc5 src/solaris/native/java/lang/java_props_md.c --- a/src/solaris/native/java/lang/java_props_md.c Thu May 21 18:41:50 2009 +0100 +++ b/src/solaris/native/java/lang/java_props_md.c Fri May 22 17:37:20 2009 +0100 @@ -211,14 +211,19 @@ * _.@ * , , and are optional. */

       char temp[64];
       char *language = NULL, *country = NULL, *variant = NULL,
            *encoding = NULL;
       char *std_language = NULL, *std_country = NULL, *std_variant = NULL,
            *std_encoding = NULL;

```
       char *p, encoding_variant[64];
```

       char *p, *encoding_variant;
       int i, found;

       char *temp = malloc(strlen(lc)+1);

```
       if (temp == NULL) {
```

           JNU_ThrowOutOfMemoryError(env, NULL);

```
           return NULL;
```
```
       }
```
#ifndef linux /* * Workaround for Solaris bug 4201684: Xlib doesn't like @euro

@@ -252,6 +257,13 @@ * to a default country if that's possible. It's also used to map * the Solaris locale aliases to their proper Java locale IDs. */ +

       encoding_variant = malloc(strlen(p)+1);

```
       if (encoding_variant == NULL) {
```

           JNU_ThrowOutOfMemoryError(env, NULL);

```
           return NULL;
```
```
       }
```

        if ((p = strchr(temp, '.')) != NULL) {
            strcpy(encoding_variant, p); /* Copy the leading '.' */
            *p = '\0';

@@ -263,7 +275,12 @@ }

         if (mapLookup(locale_aliases, temp, &p)) {

```
           strcpy(temp, p);
```

           temp = realloc(temp, strlen(p)+1);

```
           if (temp == NULL) {
```

               JNU_ThrowOutOfMemoryError(env, NULL);

```
               return NULL;
```
```
           }
```

  strcpy(temp, p);
        }

        language = temp;

@@ -357,6 +374,9 @@ #endif sprops.encoding = std_encoding; sprops.sun_jnu_encoding = sprops.encoding; +

```
       free(temp);
```
```
       free(encoding_variant);
   }
```
}

Previous message: Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties
Next message: Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

More information about the core-libs-dev mailing list