Merge pull request #74 from shogo82148/fix-isseu73

fix panics in XMLFile.readElement

Author: shogo82148

common.go

@@ -115,10 +115,16 @@ type ResValue struct {
 }
 
 // GetString returns a string referenced by ref.
+// It panics if the pool doesn't contain ref.
 func (pool *ResStringPool) GetString(ref ResStringPoolRef) string {
 	return pool.Strings[int(ref)]
 }
 
+// HasString returns whether the pool contains ref.
+func (pool *ResStringPool) HasString(ref ResStringPoolRef) bool {
+	return int(ref) >= 0 && int(ref) < len(pool.Strings)
+}
+
 func readStringPool(sr *io.SectionReader) (*ResStringPool, error) {
 	sp := new(ResStringPool)
 	if err := binary.Read(sr, binary.LittleEndian, &sp.Header); err != nil {

xml.go

@@ -12,12 +12,19 @@ import (
 // XMLFile is an XML file expressed in binary format.
 type XMLFile struct {
 	stringPool     *ResStringPool
-	resourceMap    []uint32
 	notPrecessedNS map[ResStringPoolRef]ResStringPoolRef
 	namespaces     xmlNamespaces
 	xmlBuffer      bytes.Buffer
 }
 
+type InvalidReferenceError struct {
+	Ref ResStringPoolRef
+}
+
+func (e *InvalidReferenceError) Error() string {
+	return fmt.Sprintf("androidbinary: invalid reference: 0x%08X", e.Ref)
+}
+
 type (
 	xmlNamespaces struct {
 		l []namespaceVal
@@ -30,7 +37,6 @@ type (
 
 func (x *xmlNamespaces) add(key ResStringPoolRef, value ResStringPoolRef) {
 	x.l = append(x.l, namespaceVal{key: key, value: value})
-	return
 }
 
 func (x *xmlNamespaces) remove(key ResStringPoolRef) {
@@ -41,7 +47,6 @@ func (x *xmlNamespaces) remove(key ResStringPoolRef) {
 			return
 		}
 	}
-	return
 }
 
 func (x *xmlNamespaces) get(key ResStringPoolRef) ResStringPoolRef {
@@ -164,10 +169,15 @@ func (f *XMLFile) readChunk(r io.ReaderAt, offset int64) (*ResChunkHeader, error
 }
 
 // GetString returns a string referenced by ref.
+// It panics if the pool doesn't contain ref.
 func (f *XMLFile) GetString(ref ResStringPoolRef) string {
 	return f.stringPool.GetString(ref)
 }
 
+func (f *XMLFile) HasString(ref ResStringPoolRef) bool {
+	return f.stringPool.HasString(ref)
+}
+
 func (f *XMLFile) readStartNamespace(sr *io.SectionReader) error {
 	header := new(ResXMLTreeNode)
 	if err := binary.Read(sr, binary.LittleEndian, header); err != nil {
@@ -207,12 +217,24 @@ func (f *XMLFile) readEndNamespace(sr *io.SectionReader) error {
 	return nil
 }
 
-func (f *XMLFile) addNamespacePrefix(ns, name ResStringPoolRef) string {
+func (f *XMLFile) addNamespacePrefix(ns, name ResStringPoolRef) (string, error) {
+	if !f.HasString(name) {
+		return "", &InvalidReferenceError{Ref: name}
+	}
+
 	if ns != NilResStringPoolRef {
-		prefix := f.GetString(f.namespaces.get(ns))
-		return fmt.Sprintf("%s:%s", prefix, f.GetString(name))
+		ref := f.namespaces.get(ns)
+		if ref == 0 {
+			return "", &InvalidReferenceError{Ref: ns}
+		}
+		if !f.HasString(ref) {
+			return "", &InvalidReferenceError{Ref: ref}
+		}
+		prefix := f.GetString(ref)
+
+		return fmt.Sprintf("%s:%s", prefix, f.GetString(name)), nil
 	}
-	return f.GetString(name)
+	return f.GetString(name), nil
 }
 
 func (f *XMLFile) readStartElement(sr *io.SectionReader) error {
@@ -229,7 +251,12 @@ func (f *XMLFile) readStartElement(sr *io.SectionReader) error {
 		return nil
 	}
 
-	fmt.Fprintf(&f.xmlBuffer, "<%s", f.addNamespacePrefix(ext.NS, ext.Name))
+	tag, err := f.addNamespacePrefix(ext.NS, ext.Name)
+	if err != nil {
+		return err
+	}
+	f.xmlBuffer.WriteString("<")
+	f.xmlBuffer.WriteString(tag)
 
 	// output XML namespaces
 	if f.notPrecessedNS != nil {
@@ -275,7 +302,11 @@ func (f *XMLFile) readStartElement(sr *io.SectionReader) error {
 			}
 		}
 
-		fmt.Fprintf(&f.xmlBuffer, " %s=\"", f.addNamespacePrefix(attr.NS, attr.Name))
+		name, err := f.addNamespacePrefix(attr.NS, attr.Name)
+		if err != nil {
+			return err
+		}
+		fmt.Fprintf(&f.xmlBuffer, " %s=\"", name)
 		xml.Escape(&f.xmlBuffer, []byte(value))
 		fmt.Fprint(&f.xmlBuffer, "\"")
 		offset += int64(ext.AttributeSize)
@@ -296,6 +327,10 @@ func (f *XMLFile) readEndElement(sr *io.SectionReader) error {
 	if err := binary.Read(sr, binary.LittleEndian, ext); err != nil {
 		return err
 	}
-	fmt.Fprintf(&f.xmlBuffer, "</%s>", f.addNamespacePrefix(ext.NS, ext.Name))
+	tag, err := f.addNamespacePrefix(ext.NS, ext.Name)
+	if err != nil {
+		return err
+	}
+	fmt.Fprintf(&f.xmlBuffer, "</%s>", tag)
 	return nil
 }

xml_test.go

@@ -162,12 +162,20 @@ func TestAddNamespacePrefix(t *testing.T) {
 	f.stringPool = new(ResStringPool)
 	f.stringPool.Strings = []string{"", "name", "prefix", "http://example.com"}
 
-	if actual := f.addNamespacePrefix(NilResStringPoolRef, nameRef); actual != "name" {
-		t.Errorf("got %v want name", actual)
+	got, err := f.addNamespacePrefix(NilResStringPoolRef, nameRef)
+	if err != nil {
+		t.Errorf("got %v want no error", err)
+	}
+	if got != "name" {
+		t.Errorf("got %v want name", got)
 	}
 
-	if actual := f.addNamespacePrefix(uriRef, nameRef); actual != "prefix:name" {
-		t.Errorf("got %v want prefix:name", actual)
+	got, err = f.addNamespacePrefix(uriRef, nameRef)
+	if err != nil {
+		t.Errorf("got %v want no error", err)
+	}
+	if got != "prefix:name" {
+		t.Errorf("got %v want prefix:name", got)
 	}
 }