I have Ogg files which include encoded pictures. These are large (>64k) and the code misreads the lengths of the next part and end up falling off the EOF. Looking at building a simple patch, but not yet had time.

The cmd/tag/main.so reports , for example:

$ go run main.go ~/tmp/09\ -\ The\ Boxer.ogg
error reading file: unexpected EOF

please note that this is my very first golang work, so any feedback is appreciated. Still have no clue how to prepare tests, but already questions about this:

1. are you including the necessary music files also in the repo (in case of dsf the files could be huge - the one I've tested with is 300 mb).

2. copyrights: I think some dsd files are provided for test purposes on the web, but haven't check.

When reading an MP3 with an ID3v2.4 tag, it doesn't read any of the tags.

Edit: this is what I thought, but it's not the case...

I've tracked it down to this: in the song file I have an embedded picture, which has a size that is significantly larger than the size of the header. Hence it reads one picture (which oddly returns with name == "", would actually expect APIC, right?), then offset > h.Size and the loop exits.

This all happens in the readID3v2Frames function.

This is the third vulnerability in id3v2frames.go

In readTextWithDescrFrame function, you don't check the size of b , program will happen panic when the size of b is 2 or less than 2 .

testcase 8eff69ad26a59a05ec11e38f3ca6c592f08dcc54.zip

panic: runtime error: slice bounds out of range [:3] with capacity 2

goroutine 1 [running]:
github.com/dhowden/tag.readTextWithDescrFrame(0xc0000da038, 0x3, 0x3, 0x101, 0x122b1a0, 0x0, 0x0)
        /Users/jaylin/go/pkg/mod/github.com/dhowden/[email protected]/id3v2frames.go:460 +0x46e
github.com/dhowden/tag.readID3v2Frames(0x114d680, 0xc0000d8000, 0x17, 0xc0000dc000, 0xc0000d8000, 0x0, 0xb)
        /Users/jaylin/go/pkg/mod/github.com/dhowden/[email protected]/id3v2.go:364 +0x90b
github.com/dhowden/tag.ReadID3v2Tags(0x114daa0, 0xc0000d8000, 0x1, 0x0, 0x0, 0x0)
        /Users/jaylin/go/pkg/mod/github.com/dhowden/[email protected]/id3v2.go:428 +0xde
github.com/dhowden/tag.ReadFrom(0x114daa0, 0xc0000d8000, 0xc0000d6000, 0x17, 0x217, 0x0)
        /Users/jaylin/go/pkg/mod/github.com/dhowden/[email protected]/tag.go:52 +0x324
main.main()
        /Users/jaylin/GolandProjects/gofuzz_test/main.go:20 +0xb5

Add some testing in the library.

Never had the chance to write test, so if you see something outrageous in the code, tell me.

We go up to 40,5% of test coverage.

This is another file from my library whose tag is not recognized: https://goo.gl/ov75zy

$ tag 04\ The\ Man\ With\ X-Ray\ Eyes.mp3
error reading file: invalid encoding byte 20

This file is recognized normally in a number of other tag tools. I didn't have time to dig into tag's code to figure out what is going on, but I hope the file sample can help. Thanks for this awesome library.

My environment: Mac OS X 10.11.1 Go 1.6

(Sorry for bombarding you with issues, but this is the best pure Go tag library out there and this is my way of contributing to make it even better! Thanks for your awesome job!)

I think the way that the picture is embedded in this file is breaking the library. This is what I get:

$ tag Indila.ogg
error reading file: unexpected EOF

Indila.ogg.zip

I think it may be caused by the way the picture is embedded. Normally ffmpeg reports the embedded art as a separated stream like this:

    Stream #0:1: Video: mjpeg (Baseline), yuvj444p(pc, bt470bg/unknown/unknown), 700x700 [SAR 72:72 DAR 1:1], 90k tbr, 90k tbn, 90k tbc (attached pic)
    Metadata:
      comment         : Cover (front)

But in the attached file, it appears as a tag of the audio stream:

    Stream #0:0(fra): Audio: vorbis, 44100 Hz, stereo, fltp, 320 kb/s
    Metadata:
      metadata_block_picture: AAAAAwAAAAppbWFnZS9qcGVnAAAAAAAAAAAAAAAAAAAAAAAAAAAAA5FA/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAUDBAQEAwUEBAQFBQUGBwwIBwcHBw8LCwkMEQ8SEhEPERETFhwXExQaFRERGCEYGh0dHx8fExciJCIeJBweHx7/2wBDAQUFBQcGBw4ICA4eFBEUHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4

It seems that the library is not able to get embedded pictures from OGG files. Ex from the attached file:

$ tag 01.ogg
Metadata Format: VORBIS
File Type: OGG
 Title: Over
 Album: Mer de noms
 Artist: Maynard James Keenan (lead vocals)
 Composer: Maynard James Keenan
 Genre: Alternative Rock
 Year: 0
 Track: 12 of 12
 Disc: 1 of 1
 Picture: <nil>
 Lyrics:

01.ogg.zip

Is it a bug? If not, do you have plans to support this?

The 2 offending files in a zip file: https://dl.ugjka.net/sumtest.zip

Code:

package main

import (
	"fmt"
	"os"

	"github.com/dhowden/tag"
)

func main() {
	f, _ := os.Open("test.mp3")
	f2, _ := os.Open("test2.mp3")
	fmt.Println(tag.Identify(f))
	fmt.Println(tag.Sum(f))
	fmt.Println(tag.Identify(f2))
	fmt.Println(tag.Sum(f2))
}

Output:

ID3v2.3 MP3 <nil>
533abd81b8664e1371f7013f6453d2cce5ed28f5 <nil>
ID3v2.3 MP3 <nil>
741efd6ba8e88a3d1278160b952ac4b634edc684 <nil>

The patch db723fecfc08a91c8363b422a6668e7c261bb658 is a little too strict and I end up having files with missing tag. The main cause was the iTunes specific tags.

I tweaked the validity check to just ignore unknown frame and try to continue the parsing if possible.

On a side note, the year() API call is broken in the current state, since the func (f frameNames) Name() method use the same frame names for 2.3 and 2.4 BUT in 2.4 TYER is replaced by the TDRC frame

And, to complicate things, some software still use 2.4 frames in 2.3 tagging (for example Musicbrainz Picard use TSOP frame in 2.3 tagging)

I propose to make the following change :

1. merge the validity list for 2.4 and 2.3. It will assure a better compatibility with strange tagger
2. change the logic of the Name() method to use TDRC if present, TYER if not (or maybe prefer the TORY to TYER ?)

TCON

The 'Content type', which previously was stored as a one byte numeric value only, is now a numeric string. You may use one or several of the types as ID3v1.1 did or, since the category list would be impossible to maintain with accurate and up to date categories, define your own.

References to the ID3v1 genres can be made by, as first byte, enter "(" followed by a number from the genres list (appendix A) and ended with a ")" character. This is optionally followed by a refinement, e.g. "(21)" or "(4)Eurodisco". Several references can be made in the same frame, e.g. "(51)(39)". If the refinement should begin with a "(" character it should be replaced with "((", e.g. "((I can figure out any genre)" or "(55)((I think...)". The following new content types is defined in ID3v2 and is implemented in the same way as the numerig content types, e.g. "(RX)".

I made a partial implentation as, in practice, I've only seen TCON with one numeric value.

To test it, use the id3v2 tool

% id3v2 -g 79 test.mp3 % id3v2 -l test.mp3| grep TCON TCON (Content type): Hard Rock (79) % ./tag test.mp3| grep Genre Genre: (79)

With the patch : % go build && ./tag test.mp3| grep Genre Genre: Hard Rock

Wondering if this could be implemented in the future. I'm using mctofu/musiclib and it relies on this metadata parser.

Sample files:

• example.ape
• sample1.wv

Relevant: https://github.com/mctofu/musiclib/issues/30

Thanks!

I copied an mp3 file and changed the tag info (album, title, and artist), and hashed the origin and the copy, and they came up as different values.

The format of the files was ID3v2.3

I encountered this issue with my program alto where I was expecting the %artist% variable to return m.c["artist"], which is the actual artist field defined within the metadata, but it returned m.c["performer"] instead. Unbeknownst to me, I thought this was an error with my code, so after a few minutes of trial and error still yielded the same result. I was puzzled, but then decided to check Windows Explorer and said that the artist field was defined. I was confused at why it wasn't returning m.c["artist"] when Windows Explorer was, but that was when I finally decided to conclude my confusion by going directly to the source only to see the following code:

https://github.com/dhowden/tag/blob/master/vorbis.go#L175-L185

func (m *metadataVorbis) Artist() string {
	// PERFORMER
	// The artist(s) who performed the work. In classical music this would be the
	// conductor, orchestra, soloists. In an audio book it would be the actor who
	// did the reading. In popular music this is typically the same as the ARTIST
	// and is omitted.
	if m.c["performer"] != "" {
		return m.c["performer"]
	}
	return m.c["artist"]
}

It turned out that m.c["performer"] was " " which caused the library to return it instead.

ID3v1 and ID3v2 doesn't do this, so why does Vorbis? Metadata for each audio file varies, and when I call a method like Artist, I expect m.c["artist"], not m.c["performer"].

I was originally thinking about using Metadata#Raw() to mitigate this, but after seeing the note that says that it is an unstable mapping, which was expected, I couldn't risk instability for my program.

Hi, I was wondering if the tag cmd could have an option like tag foo.flac --title to only print the value, and not the rest of the keys, or values? That would come very handy for various shellscripts, I think.