mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-15 15:16:13 +00:00
fix(ding): clamp sample rate against int -> uint32 truncation
CodeQL flagged the writeWAV cast of strconv.Atoi's result to uint32 (alert 148). Two-layer defence: the handler rejects sample-rate query params outside [8000, 192000] before parsing ever reaches Render, and WithDefaults snaps any out-of-range caller-supplied SampleRate back to the default before renderChirp allocates buffers sized by it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
4f6f4c497a
commit
b149580c19
@@ -79,7 +79,7 @@ func DefaultOptions() Options {
|
||||
func (o Options) WithDefaults() Options {
|
||||
d := DefaultOptions()
|
||||
|
||||
if o.SampleRate <= 0 {
|
||||
if o.SampleRate <= 0 || o.SampleRate > int(maxSampleRate) {
|
||||
o.SampleRate = d.SampleRate
|
||||
}
|
||||
|
||||
@@ -151,7 +151,13 @@ func Render(opts Options) []byte {
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
_ = writeWAV(&buf, left, right, opts.SampleRate)
|
||||
// Defensive bound check: clamp before the conversion to
|
||||
// uint32 so even a buggy caller (or one that bypassed the
|
||||
// handler-side bound check on the query param) can't trigger
|
||||
// integer truncation in the WAV header fields.
|
||||
sampleRate32 := safeSampleRate(opts.SampleRate)
|
||||
|
||||
_ = writeWAV(&buf, left, right, sampleRate32)
|
||||
|
||||
return buf.Bytes()
|
||||
}
|
||||
@@ -225,7 +231,28 @@ const (
|
||||
wavBitsPer = 16
|
||||
)
|
||||
|
||||
func writeWAV(w io.Writer, left, right []float64, sampleRate int) error {
|
||||
// maxSampleRate is the largest sample rate writeWAV will accept
|
||||
// before clamping. Generous enough to allow studio-quality 192
|
||||
// kHz; well below the uint32 ceiling the WAV header field can
|
||||
// represent, and far below anything the byte-rate multiplication
|
||||
// downstream could overflow.
|
||||
const maxSampleRate uint32 = 192_000
|
||||
|
||||
// safeSampleRate converts the operator-supplied int sample rate
|
||||
// into the uint32 the WAV header needs, clamping anything
|
||||
// out-of-range to the default. Defence-in-depth: the
|
||||
// handler-side sampleRateParam already rejects unreasonable
|
||||
// inputs, but Render is exported so other callers (tests,
|
||||
// scripts) could pass anything.
|
||||
func safeSampleRate(in int) uint32 {
|
||||
if in <= 0 || in > int(maxSampleRate) {
|
||||
return uint32(DefaultOptions().SampleRate)
|
||||
}
|
||||
|
||||
return uint32(in)
|
||||
}
|
||||
|
||||
func writeWAV(w io.Writer, left, right []float64, sampleRate uint32) error {
|
||||
if len(left) != len(right) {
|
||||
return fmt.Errorf("channel length mismatch: %d vs %d", len(left), len(right))
|
||||
}
|
||||
@@ -262,11 +289,11 @@ func writeWAV(w io.Writer, left, right []float64, sampleRate int) error {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := binary.Write(w, binary.LittleEndian, uint32(sampleRate)); err != nil {
|
||||
if err := binary.Write(w, binary.LittleEndian, sampleRate); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
byteRate := uint32(sampleRate * wavChannels * (wavBitsPer / 8))
|
||||
byteRate := sampleRate * uint32(wavChannels) * uint32(wavBitsPer/8)
|
||||
if err := binary.Write(w, binary.LittleEndian, byteRate); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -77,6 +77,41 @@ func TestRender_OverrideSampleRateChangesByteRate(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSafeSampleRate_ClampsOutOfRange(t *testing.T) {
|
||||
cases := []struct {
|
||||
in int
|
||||
want uint32
|
||||
}{
|
||||
{22050, 22050},
|
||||
{44100, 44100},
|
||||
{192000, 192000},
|
||||
{0, 22050}, // zero → default
|
||||
{-1, 22050}, // negative → default
|
||||
{200000, 22050}, // above max → default
|
||||
{1 << 31, 22050}, // way beyond uint32 → default (the original CodeQL concern)
|
||||
{1 << 33, 22050}, // wraps to a different value on int→uint32; default protects us
|
||||
{int(maxSampleRate) + 1, 22050},
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
if got := safeSampleRate(c.in); got != c.want {
|
||||
t.Errorf("safeSampleRate(%d) = %d, want %d", c.in, got, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRender_HugeSampleRateDoesNotTruncateOrPanic(t *testing.T) {
|
||||
// Regression for the int→uint32 truncation CodeQL flagged.
|
||||
// A caller (test, future SDK user) bypassing the handler's
|
||||
// sampleRateParam guard with an int well above uint32 used
|
||||
// to silently wrap. The defensive clamp now substitutes the
|
||||
// default sample rate instead.
|
||||
data := Render(Options{SampleRate: 1 << 33}.WithDefaults())
|
||||
if sr := binary.LittleEndian.Uint32(data[24:28]); sr != uint32(DefaultOptions().SampleRate) {
|
||||
t.Errorf("expected clamp to default sample rate, got %d", sr)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWithDefaults_FillsZeroFields(t *testing.T) {
|
||||
got := Options{PitchHigh: 1000}.WithDefaults()
|
||||
if got.PitchHigh != 1000 {
|
||||
|
||||
@@ -114,7 +114,7 @@ func parseDingOptions(r *http.Request) (ding.Options, bool) {
|
||||
touched = true
|
||||
}
|
||||
|
||||
if v, ok := intParam(q.Get("sample-rate")); ok {
|
||||
if v, ok := sampleRateParam(q.Get("sample-rate")); ok {
|
||||
opts.SampleRate = v
|
||||
touched = true
|
||||
}
|
||||
@@ -157,13 +157,23 @@ func millisecondsParam(raw string) (float64, bool) {
|
||||
return float64(v) / 1000.0, true
|
||||
}
|
||||
|
||||
func intParam(raw string) (int, bool) {
|
||||
// dingMinSampleRate / dingMaxSampleRate gate the operator-supplied
|
||||
// sample-rate against int→uint32 truncation and against values
|
||||
// outside any realistic audio range. The upper bound is generous
|
||||
// (192 kHz is studio-quality) but well below the uint32 ceiling
|
||||
// the WAV header field can hold.
|
||||
const (
|
||||
dingMinSampleRate = 8000
|
||||
dingMaxSampleRate = 192000
|
||||
)
|
||||
|
||||
func sampleRateParam(raw string) (int, bool) {
|
||||
if raw == "" {
|
||||
return 0, false
|
||||
}
|
||||
|
||||
v, err := strconv.Atoi(raw)
|
||||
if err != nil || v <= 0 {
|
||||
if err != nil || v < dingMinSampleRate || v > dingMaxSampleRate {
|
||||
return 0, false
|
||||
}
|
||||
|
||||
|
||||
@@ -90,6 +90,35 @@ func TestHandleDing_InvalidParamFallsBackToDefault(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleDing_OutOfRangeSampleRateFallsBackToDefault(t *testing.T) {
|
||||
// CodeQL flagged the int→uint32 truncation; reject values
|
||||
// outside the sane audio range at the parser, so neither
|
||||
// the WAV header nor the int→uint32 cast can be tricked.
|
||||
cases := []string{
|
||||
"1", // below dingMinSampleRate
|
||||
"7999", // just under the floor
|
||||
"500000", // above dingMaxSampleRate
|
||||
"4294967300", // > uint32 — the truncation source
|
||||
}
|
||||
|
||||
ts := newDingTestServer(t)
|
||||
|
||||
for _, sr := range cases {
|
||||
t.Run(sr, func(t *testing.T) {
|
||||
res, err := http.Get(ts.URL + "/media/aftertouch-ding.wav?sample-rate=" + sr)
|
||||
if err != nil {
|
||||
t.Fatalf("GET: %v", err)
|
||||
}
|
||||
defer res.Body.Close()
|
||||
|
||||
body := readAll(t, res)
|
||||
if got := binary.LittleEndian.Uint32(body[24:28]); got != 22050 {
|
||||
t.Errorf("sample-rate=%s should clamp to default 22050, got %d", sr, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleDing_DefaultIsCached(t *testing.T) {
|
||||
// Reset the cache by simulating a fresh process.
|
||||
dingDefaultCache.once = sync.Once{}
|
||||
|
||||
Reference in New Issue
Block a user