RFR (M): 8161224: CONSTANT_NameAndType_info permits references to illegal names and descriptors

Dmitry Dmitriev dmitry.dmitriev at oracle.com
Tue Sep 6 20:29:40 UTC 2016


Looks good! 

Thanks, 
Dmitry 
----- Original Message ----- 
From: rachel.protacio at oracle.com 
To: dmitry.dmitriev at oracle.com 
Cc: hotspot-runtime-dev at openjdk.java.net 
Sent: Tuesday, September 6, 2016 11:23:11 PM GMT +03:00 Iraq 
Subject: Re: RFR (M): 8161224: CONSTANT_NameAndType_info permits references to illegal names and descriptors 


That sounds fair to me. How's this? 

@@ -4843,19 +4840,28 @@ 
} 
} 
else { 
- // 4900761: For class version > 48, any unicode is allowed in class name. 
+ // Skip leading 'L' and ignore first appearance of ';' 
length--; 
signature++; 
- while (length > 0 && signature[0] != ';') { 
- if (signature[0] == '.') { 
- classfile_parse_error("Class name contains illegal character '.' in descriptor in class 
file %s", CHECK_0); 
- } 
- length--; 
- signature++; 
- } 
- if (signature[0] == ';') { return signature + 1; } 
- } 
- 
+ char* c = strchr((char*) signature, ';'); 
+ // Format check signature 
+ if (c != NULL) { 
+ ResourceMark rm(THREAD); 
+ int newlen = c - (char*) signature; 
+ char* sig = NEW_RESOURCE_ARRAY(char, newlen + 1); 
+ strncpy(sig, signature, newlen); 
+ sig[newlen] = '\0'; 
+ 
+ bool legal = verify_unqualified_name(sig, newlen, LegalClass); 
+ if (!legal) { 
+ classfile_parse_error("Class name contains illegal character " 
+ "in descriptor in class file %s", 
+ CHECK_0); 
+ return NULL; 
+ } 
+ return signature + newlen + 1; 
+ } 
+ } 
return NULL; 
} 
case JVM_SIGNATURE_ARRAY: 

It builds fine and passes the jck tests in question. 
Rachel 


On 9/6/2016 4:14 PM, Dmitry Dmitriev wrote: 


Hi Rachel,

Small question about resource mark: on line 4850 you allocate new resource array which is used only in verify_unqualified_name() call. Do we need resource mark in this block?

Thanks,
Dmitry

----- Original Message -----
From: rachel.protacio at oracle.com To: hotspot-runtime-dev at openjdk.java.net Sent: Tuesday, September 6, 2016 11:01:45 PM GMT +03:00 Iraq
Subject: Re: RFR (M): 8161224: CONSTANT_NameAndType_info permits references to illegal names and descriptors

Hi Harold,

Thanks for the review! I've removed the resource mark. If no further 
comments in the next hour, will commit.

Rachel

On 9/6/2016 3:29 PM, harold seigel wrote: 

Hi Rachel,

The change looks good.  Just one nit:

No resource mark is needed before calling classfile_parse_error() at 
line 4857.

(No new RFR needed.)

Thanks, Harold


On 9/1/2016 5:30 PM, Rachel Protacio wrote: 

Hello!

Please review this fix, which addresses a few issues related to 
incomplete format checking with NameAndType names and signatures.

First, the code that should have been format checking the strings in 
later classfile versions was in fact just checking for periods, so 
I've rewritten it to call verify_unqualified_name(). Second, the 
checks were (depending on the version) only performed when referenced 
through Methodref/Fieldref/InterfaceMethodref/InvokeDynamic, meaning 
that non-referenced NameAndType bytecodes did not get checked like 
they were supposed to. My change enforces the spec in both aspects. 
To summarize:

The existing code had:
- strict checks for pre-5.0
- incomplete/non-spec-compliant checks for 5.0-and-later
- no checks for un-referenced NameAndType (A) names and (B) 
6.0-and-earlier signatures.

My change has:
- the same strict checks for pre-5.0
- complete/spec-compliant checks for 5.0-and-later
- all checks moved to the NameAndType section so all names and 
signatures will be checked regardless of whether NameAndType is 
referenced.

Bug: https://bugs.openjdk.java.net/browse/JDK-8161224 Open webrev: http://cr.openjdk.java.net/~rprotacio/8161224.00/ Testing: The jck tests which had been failing for this bug now pass, 
along with all other jck vm tests. Also tested with JPRT and RBT 
hotspot_all and noncolo tests.

A compatibility request has been approved for this change.

Thank you,
Rachel 


More information about the hotspot-runtime-dev mailing list