Re: RFR: 8282578: AIOOBE in javax.sound.sampled.Clip [v2]

[Alexander Zvegintsev]

On Sun, 4 Dec 2022 22:17:55 GMT, Alexander Zuev <kiz...@openjdk.org> wrote:

>> Add try/catch clause to ignore an exception since it is harmless for we 
>> isolated
>> the massge data before passing it ro processor.
>> Add test case.
>
> 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 166:

> 164:                                         break;
> 165:                                     }
> 166:                                     // 
> http://www.midi.org/about-midi/tuning_extens.shtml

Unrelated to change, but looks like those links become unavailable years ago.

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.

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`

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`

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`?

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]);
}

-------------

PR: https://git.openjdk.org/jdk/pull/9016