From 645183c903974b2f75adf82508d1ba25bc4ae14f Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Tue, 26 Oct 2021 21:57:28 +0530 Subject: [PATCH] Fix OOB Vibration Array Access in `VibrateDevice` Access to the `vibrations` field in `vibrations[3].period` could lead to UB, this has been replaced with a proper check which adds up the period over all vibrations instead. A minor cleanup with variable names and explicit types for integer arithmetic has also been done. --- .../main/cpp/skyline/input/npad_device.cpp | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/app/src/main/cpp/skyline/input/npad_device.cpp b/app/src/main/cpp/skyline/input/npad_device.cpp index 335ce889..9c94bed8 100644 --- a/app/src/main/cpp/skyline/input/npad_device.cpp +++ b/app/src/main/cpp/skyline/input/npad_device.cpp @@ -375,12 +375,15 @@ namespace skyline::input { return a.period < b.period; }); + jlong totalPeriod{}; jint totalAmplitude{}; - for (const auto &vibration : vibrations) + for (const auto &vibration : vibrations) { + totalPeriod += vibration.period; totalAmplitude += vibration.amplitude; + } // If this vibration is essentially null then we don't play rather clear any running vibrations - if (totalAmplitude == 0 || vibrations[3].period == 0) { + if (totalPeriod == 0 || totalAmplitude == 0) { jvm->ClearVibrationDevice(index); return; } @@ -390,13 +393,13 @@ namespace skyline::input { std::array amplitudes; // We are essentially unrolling the bands into a linear sequence, due to the data not being always linearizable there will be inaccuracies at the ends unless there's a pattern that's repeatable which will happen when all band's frequencies are factors of each other - u8 i{}; - for (; i < timings.size(); i++) { + size_t timingIndex{}; + for (; timingIndex < timings.size(); timingIndex++) { jlong time{}; - u8 startCycleCount{}; - for (u8 n{}; n < vibrations.size(); n++) { - auto &vibration{vibrations[n]}; + size_t startCycleCount{}; + for (size_t vibrationIndex{}; vibrationIndex < vibrations.size(); vibrationIndex++) { + auto &vibration{vibrations[vibrationIndex]}; if (totalTime <= vibration.start) { vibration.start = vibration.end + vibration.period; totalAmplitude += vibration.amplitude; @@ -410,16 +413,16 @@ namespace skyline::input { } // If all bands start again at this point then we can end the pattern here as a loop to the front will be flawless - if (i && startCycleCount == vibrations.size()) + if (timingIndex && startCycleCount == vibrations.size()) break; - timings[i] = time; + timings[timingIndex] = time; totalTime += time; - amplitudes[i] = std::min(totalAmplitude, AmplitudeMax); + amplitudes[timingIndex] = std::min(totalAmplitude, AmplitudeMax); } - jvm->VibrateDevice(index, span(timings.begin(), timings.begin() + i), span(amplitudes.begin(), amplitudes.begin() + i)); + jvm->VibrateDevice(index, span(timings.begin(), timings.begin() + timingIndex), span(amplitudes.begin(), amplitudes.begin() + timingIndex)); } void VibrateDevice(const std::shared_ptr &jvm, i8 index, const NpadVibrationValue &value) { @@ -461,7 +464,7 @@ namespace skyline::input { {right.frequencyLow, right.amplitudeLow * (AmplitudeMax / 4)}, {right.frequencyHigh, right.amplitudeHigh * (AmplitudeMax / 4)}, }; - VibrateDevice<4>(manager.state.jvm, index, vibrations); + VibrateDevice(manager.state.jvm, index, vibrations); } else { VibrateDevice(manager.state.jvm, index, left); VibrateDevice(manager.state.jvm, partnerIndex, right);