Skip to content

Commit 45f066a

Browse files
committed
fix: prevent OOM on malformed APK with crafted string table
Two allocation vulnerabilities in string table parsing allow a crafted resources.arsc to cause multi-GiB heap allocations: parseStringTable — res.data = make([]byte, r.N) r.N is derived from the chunk's totalLen field, a raw uint32 that a malformed APK can set to ~4 GiB. make() commits the memory immediately; io.ReadFull then fails (if the file is smaller), but only after the 4 GiB block is live on the heap. The existing stringCnt >= 2M guard does not bound the data section size. Guard with a 50 MiB limit before the make. parseString16 — buf = make([]uint16, strCharacters) The extended 31-bit length encoding (high bit of the first uint16 set) allows strCharacters up to 0x7FFF_FFFF (4 GiB as []uint16) per call. r is bytes.NewReader(t.data[offset:]), so binary.Read fails immediately when the actual data is smaller -- but only after the allocation lands. Fix with two guards in order: 1. Validate strCharacters against br.Len(): a correctly-formed string cannot claim more chars than remaining_bytes/2. Catches both crafted fields and any future decoding misalignment. 2. Secondary cap at 64 KiB: no legitimate resource string exceeds this. Also fix &buf -> buf in binary.Read calls (parseString16, parseString8): Passing *[]uint16 / *[]uint8 instead of the slice value itself bypasses binary.Read's intDataSize fast-path switch (which handles []uint16 / []uint8 but not pointer-to-slice). The fallback reflection path allocates an extra []byte temp buffer of the same size and calls reflect.New per element -- visible as ~3 GiB flat in encoding/binary.Read and ~378 MB in reflect.New in heap profiles of affected servers. Passing the slice directly uses the fast path, is functionally identical, and is zero- reflection.
1 parent 7294e27 commit 45f066a

1 file changed

Lines changed: 36 additions & 4 deletions

File tree

stringtable.go

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,15 @@ func parseStringTable(r *io.LimitedReader) (stringTable, error) {
101101
}
102102
}
103103

104+
// A string table chunk's totalLen is a uint32, so r.N can reach ~4 GiB on a
105+
// malformed APK. Guard before the allocation: the make succeeds (if the system
106+
// has the memory) and io.ReadFull then fails, but the 4 GiB is already live on
107+
// the heap. 50 MiB covers every real-world app.
108+
const maxStringTableData = 50 * 1024 * 1024
109+
if r.N > maxStringTableData {
110+
return res, fmt.Errorf("string table data section too large: %d bytes", r.N)
111+
}
112+
104113
res.data = make([]byte, r.N)
105114
if _, err := io.ReadFull(r, res.data); err != nil {
106115
return res, fmt.Errorf("Failed to read string table data: %s", err.Error())
@@ -128,9 +137,32 @@ func (t *stringTable) parseString16(r io.Reader) (string, error) {
128137
strCharacters = uint32(strCharactersHigh)
129138
}
130139

131-
buf := make([]uint16, int64(strCharacters))
132-
if err := binary.Read(r, binary.LittleEndian, &buf); err != nil {
133-
return "", fmt.Errorf("error reading string : %s", err.Error())
140+
// Validate strCharacters against the bytes still available in the reader before
141+
// allocating. get() always passes bytes.NewReader(t.data[offset:]), so the
142+
// assertion is guaranteed to succeed; it also guards against future call-site
143+
// changes. A correctly-formed UTF-16 string cannot claim more characters than
144+
// remaining_bytes/2, so this catches both crafted length fields and any decoding
145+
// misalignment.
146+
if br, ok := r.(*bytes.Reader); ok {
147+
if uint64(strCharacters)*2 > uint64(br.Len()) {
148+
return "", fmt.Errorf("string length %d chars exceeds available data %d bytes",
149+
strCharacters, br.Len())
150+
}
151+
}
152+
153+
// Secondary cap: no legitimate resource string needs more than 64 KiB characters.
154+
const maxStringChars = 64 * 1024
155+
if strCharacters > maxStringChars {
156+
return "", fmt.Errorf("string too long: %d characters", strCharacters)
157+
}
158+
159+
buf := make([]uint16, strCharacters)
160+
// Pass buf ([]uint16) not &buf (*[]uint16). binary.Read's intDataSize fast-path
161+
// switch handles []uint16 but not *[]uint16; passing a pointer-to-slice silently
162+
// falls into the reflection path, allocating an extra []byte temp buffer of the
163+
// same size and calling reflect.New per element.
164+
if err := binary.Read(r, binary.LittleEndian, buf); err != nil {
165+
return "", fmt.Errorf("error reading string: %s", err.Error())
134166
}
135167

136168
decoded := utf16.Decode(buf)
@@ -173,7 +205,7 @@ func (t *stringTable) parseString8(r io.Reader) (string, error) {
173205
}
174206

175207
buf := make([]uint8, len8)
176-
if err := binary.Read(r, binary.LittleEndian, &buf); err != nil {
208+
if err := binary.Read(r, binary.LittleEndian, buf); err != nil {
177209
return "", fmt.Errorf("error reading string : %s", err.Error())
178210
}
179211

0 commit comments

Comments
 (0)