On Wed, 7 Dec 2022 15:15:34 GMT, Alexander Zvegintsev <[email protected]> wrote:
>> Alexander Zuev has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains five additional >> commits since the last revision: >> >> - Got rid of the try/catch and using the proper per-case data length >> analysis. >> - Merge branch 'master' into JDK-8282578 >> - Merge branch 'master' of https://github.com/azuev-java/jdk into >> JDK-8282578 >> - Merge branch 'master' >> - 8282578: AIOOBE in javax.sound.sampled.Clip >> >> Surround SysEx message processing block with try/catch allowing MIDI >> subsystem to >> attempt to ingest the rest of the file. >> >> Added test case. > > src/java.desktop/share/classes/com/sun/media/sound/SoftMainMixer.java line > 406: > >> 404: int ix = 0; >> 405: for (int j = 6; j < data.length - >> 1; j += 2) { >> 406: destinations[ix] = data[j] & >> 0xFF; > > Here is another possible AIOOOBE > > e.g. if `data` length is 8, we will have `destination` and `ranges` length of > 0: > > > int[] data = new int[8]; > > System.out.println("data len " + data.length); > if (data.length < 7) { > System.out.println("Prevent"); > return; > } > > int newSize = (data.length - 7) / 2; > System.out.println("new size " + newSize); > > int[] destinations = new int[newSize]; > int[] ranges = new int[newSize]; > int ix = 0; > for (int j = 6; j < data.length - 1; j += 2) { > System.out.println("index %d %d".formatted(j, ix) ); > destinations[ix] = data[j] & 0xFF; > System.out.println("index " + (j + 1)); > ranges[ix] = data[j + 1] & 0xFF; > ix++; > } > > > Same applies to similar cases below. Ok, i think there are 3 places total with this possibility when increment goes by 2 so fixed them all. > src/java.desktop/share/classes/com/sun/media/sound/SoftMainMixer.java line > 462: > >> 460: case 0x0A: // Key Based Instrument Control >> 461: { >> 462: if (data.length < 7 || (data[4] & 0xFF) != >> 01) { > > Neat: `0x01` Ok. > src/java.desktop/share/classes/com/sun/media/sound/SoftTuning.java line 114: > >> 112: //if (!checksumOK2(data)) >> 113: // break; >> 114: if (data.length < 406) { > > At first glance it looks like a magic numbers to me.<br> > for better readability we could declare `r` before this check and use `128*3 > + r` Makes sense, fixed. > src/java.desktop/share/classes/com/sun/media/sound/SoftTuning.java line 136: > >> 134: } >> 135: int ll = data[6] & 0xFF; >> 136: if (data.length < ll * 4 + 8) { > > Shouldn't it be `ll * 4 + 7` == `ll * 4 + r`? Yes. > src/java.desktop/share/classes/com/sun/media/sound/SoftTuning.java line 231: > >> 229: { >> 230: // http://www.midi.org/about-midi/tuning-scale.shtml >> 231: if (data.length < 21) { > > `< 20`? > > > int[] data = new int[20]; > for (int i = 0; i < 12; i++) { > System.out.println(data[i + 8]); > } Yes. Math is hard. ------------- PR: https://git.openjdk.org/jdk/pull/9016
