From 7bb5f7fde2f9917ea3753b09d86cb181ef71cf7a Mon Sep 17 00:00:00 2001 From: Petteri Hintsanen Date: Sun, 21 Feb 2021 18:52:55 +0200 Subject: Improve id3v2 validity checks - Remove id3v2 size checks agains emms-info-native--max-peek-size. Decoded sizes do not guide memory reservation anymore so checks are in that sense redundant. Trigger errors on zero-sized elements though, because they are always wrong. - Verify frame identifiers. --- emms-info-native.el | 88 +++++++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 40 deletions(-) diff --git a/emms-info-native.el b/emms-info-native.el index 039738c..bca21b1 100644 --- a/emms-info-native.el +++ b/emms-info-native.el @@ -519,11 +519,13 @@ outside itself.") (revision u8) (flags bits 1) (size-bytes vec 4) - (size eval (emms-info-native--decode-id3v2-size last t))) + (size eval (emms-info-native--checked-id3v2-size 'tag last))) "id3v2 header specification.") (defconst emms-info-native--id3v2-frame-header-bindat-spec '((id str (eval (if (= emms-info-native--id3v2-version 2) 3 4))) + (eval (unless (emms-info-native--valid-id3v2-frame-id-p last) + (error "id3v2 frame id ‘%s’ is invalid" last))) (size-bytes vec (eval (if (= emms-info-native--id3v2-version 2) 3 4))) (size eval (emms-info-native--checked-id3v2-size 'frame last)) (flags bits (eval (if (= emms-info-native--id3v2-version 2) 0 2)))) @@ -569,7 +571,7 @@ in case of errors or if there were no known fields in FILENAME. See ‘emms-info-native--id3v2-frame-to-info’ for recognized fields." - (condition-case-unless-debug nil + (condition-case nil (let* (emms-info-native--id3v2-version (header (emms-info-native--decode-id3v2-header filename)) (tag-size (bindat-get-field header 'size)) @@ -578,7 +580,7 @@ fields." (when (memq 6 (bindat-get-field header 'flags)) ;; Skip the extended header. (cl-incf offset - (emms-info-native--decode-id3v2-ext-header-size filename))) + (emms-info-native--checked-id3v2-ext-header-size filename))) (emms-info-native--decode-id3v2-frames filename offset (+ tag-size 10) @@ -593,10 +595,9 @@ fields." (bindat-unpack emms-info-native--id3v2-header-bindat-spec (buffer-string)))) -(defun emms-info-native--decode-id3v2-ext-header-size (filename) +(defun emms-info-native--checked-id3v2-ext-header-size (filename) "Read and decode id3v2 extended header size from FILENAME. -Return the size. Signal an error if the size exceeds -‘emms-info-native--max-peek-size’." +Return the size. Signal an error if the size is zero." (with-temp-buffer (set-buffer-multibyte nil) (insert-file-contents-literally filename nil 10 14) @@ -604,18 +605,18 @@ Return the size. Signal an error if the size exceeds (defun emms-info-native--checked-id3v2-size (elt bytes) "Calculate id3v2 element ELT size from BYTES. -ELT must be either 'tag or 'frame. Check the validity of size. -Return the size." - (let (size) - (cond ((eq elt 'tag) - (setq size (emms-info-native--decode-id3v2-size bytes t))) - ((eq elt 'frame) - (if (= emms-info-native--id3v2-version 4) - (setq size (emms-info-native--decode-id3v2-size bytes t)) - (setq size (emms-info-native--decode-id3v2-size bytes nil))))) - (when (> size emms-info-native--max-peek-size) - (error "id3v2 tag or frame size %s is invalid" size)) - size)) +ELT must be either 'tag or 'frame. + +Return the size. Signal an error if the size is zero." + (let ((size (cond ((eq elt 'tag) + (emms-info-native--decode-id3v2-size bytes t)) + ((eq elt 'frame) + (if (= emms-info-native--id3v2-version 4) + (emms-info-native--decode-id3v2-size bytes t) + (emms-info-native--decode-id3v2-size bytes nil)))))) + (if (zerop size) + (error "id3v2 tag/frame size is zero") + size))) (defun emms-info-native--decode-id3v2-size (bytes syncsafe) "Decode id3v2 element size from BYTES. @@ -641,33 +642,40 @@ Return metadata in a list of (FIELD . VALUE) cons cells." (let ((offset begin) (limit (- end (emms-info-native--id3v2-frame-header-size))) comments) - (catch 'eof - (while (< offset limit) - (let* ((header (emms-info-native--decode-id3v2-frame-header filename - offset)) - (info-id (emms-info-native--id3v2-frame-info-id header)) - (decoded-size (bindat-get-field (cdr header) 'size))) - (when (= decoded-size 0) (throw 'eof t)) - (setq offset (car header)) ;advance to frame data begin - (if (or unsync info-id) - ;; Note that if unsync is t, we have to always read a - ;; frame to gets its true size so that we can adjust - ;; offset correctly. - (let ((data (emms-info-native--read-id3v2-frame-data filename - offset - decoded-size - unsync))) - (setq offset (car data)) - (when info-id - (let ((value (emms-info-native--decode-id3v2-string (cdr data)))) - (push (cons info-id value) comments)))) - ;; Skip the frame. - (cl-incf offset decoded-size))))) + (condition-case nil + (while (< offset limit) + (let* ((header (emms-info-native--decode-id3v2-frame-header filename + offset)) + (info-id (emms-info-native--id3v2-frame-info-id header)) + (decoded-size (bindat-get-field (cdr header) 'size))) + (setq offset (car header)) ;advance to frame data begin + (if (or unsync info-id) + ;; Note that if unsync is t, we have to always read a + ;; frame to gets its true size so that we can adjust + ;; offset correctly. + (let ((data (emms-info-native--read-id3v2-frame-data filename + offset + decoded-size + unsync))) + (setq offset (car data)) + (when info-id + (let ((value (emms-info-native--decode-id3v2-string (cdr data)))) + (push (cons info-id value) comments)))) + ;; Skip the frame. + (cl-incf offset decoded-size)))) + (error nil)) comments)) (defun emms-info-native--id3v2-frame-header-size () + "Return the last decoded header size in bytes." (if (= emms-info-native--id3v2-version 2) 6 10)) +(defun emms-info-native--valid-id3v2-frame-id-p (id) + "Return t if ID is a proper id3v2 frame identifier, nil otherwise." + (if (= emms-info-native--id3v2-version 2) + (string-match "[A-Z0-9]\\{3\\}" id) + (string-match "[A-Z0-9]\\{4\\}" id))) + (defun emms-info-native--decode-id3v2-frame-header (filename begin) "Read and decode id3v2 frame header from FILENAME. Start reading from offset BEGIN. -- cgit v1.2.3