From 1f8fbba0bc5da3e5601b1d8285022cb20fe8287b Mon Sep 17 00:00:00 2001
From: Steve Tynor <chinenual@gmail.com>
Date: Mon, 25 Apr 2022 15:19:41 -0400
Subject: [PATCH] fixes #73 - checksums are sanity checked when reading SYN
 file disk

---
 RELEASE_NOTES.md     |  3 ++-
 VERSION              |  2 +-
 seq/syn2midi.go      | 10 +++++++++-
 synio/synio_files.go | 45 +++++++++++++++++++++++---------------------
 version.go           |  2 +-
 5 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 8665396..dbd84be 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -13,7 +13,8 @@ permalink: /docs/release-notes
 
 * Adds the ability to convert the sequencer events from a SYN file into a MIDI file.  See
  [SYN to MIDI Converter](https://chinenual.github.io/synergize/docs/syn2midi)
-
+* Fixes [issue #73](https://github.com/chinenual/synergize/issues/73):
+  SYN files are sanity checked (via its embedded checksum) before loading into the instrument or attempting to convert to MIDI.
 ## 2.5.0
 
 * Adds the ability convert Yamaha DX7 SYX files to Synergy CRT and VCE
diff --git a/VERSION b/VERSION
index d9ccbaa..56f6156 100644
--- a/VERSION
+++ b/VERSION
@@ -1,2 +1,2 @@
-VERSION=2.6.0-beta1
+VERSION=2.6.0-beta2
 
diff --git a/seq/syn2midi.go b/seq/syn2midi.go
index a59d462..39b1069 100644
--- a/seq/syn2midi.go
+++ b/seq/syn2midi.go
@@ -6,12 +6,15 @@ import (
 	"math"
 	"time"
 
+	"github.com/chinenual/synergize/synio"
+
 	"gitlab.com/gomidi/midi/v2"
 	"gitlab.com/gomidi/midi/v2/smf"
 
 	"github.com/chinenual/synergize/logger"
 
 	"github.com/chinenual/synergize/data"
+	"github.com/pkg/errors"
 )
 
 // See INTF.Z80 TSTATE   and SEQREQ.Z80
@@ -33,6 +36,11 @@ func GetSYNSequencerState(path string) (trackButtons [4]TrackPlayMode, err error
 		return
 	}
 
+	if err = synio.CheckSYNCRC(synBytes); err != nil {
+		err = errors.Wrap(err, "Invalid SYN file - invalid checksum")
+		return
+	}
+
 	cmosLen := data.BytesToWord(synBytes[1], synBytes[0])
 	logger.Debugf("CMOS LEN: %v\n", cmosLen)
 
@@ -639,7 +647,7 @@ func seqVelocityToSynergy(v uint8) uint8 {
 // MIDI velocity to native Synergy velocity (used by unit tests)
 // emulate what the firmware does
 func midiVelocityToSynergy(v uint8) uint8 {
-	// MIDI.Z80 convers MIDI velocity in opposite way: (v / 4 + 1)
+	// MIDI.Z80 converts MIDI velocity in opposite way: (v / 4 + 1)
 	//	YMKEYD:
 	//  ...
 	//				LD	A,(HL)			;Get midi velocity
diff --git a/synio/synio_files.go b/synio/synio_files.go
index 2da8f7b..744038d 100644
--- a/synio/synio_files.go
+++ b/synio/synio_files.go
@@ -168,6 +168,10 @@ func LoadSYN(bytes []byte) (err error) {
 	if synioVerbose {
 		logger.Infof("SYNIO: ** LoadSYN\n")
 	}
+	if err = CheckSYNCRC(bytes); err != nil {
+		err = errors.Wrap(err, "Invalid SYN file - invalid checksum")
+		return
+	}
 	if err = command(OP_STLOAD, "STLOAD"); err != nil {
 		return
 	}
@@ -245,31 +249,30 @@ func SaveSYN() (bytes []byte, err error) {
 		return
 	}
 
-	// FIXME: these bytes seem out of order vs the length HOB/LOB yet seem to be transmitted the same
-	// from INTF.Z80 firmware sourcecode - I dont understand something..
-	crcFromSynergy := data.BytesToWord(crc_buf[0], crc_buf[1])
-
-	crcHash.Reset()
+	bytes = append(len_buf, cmos_buf...)
+	bytes = append(bytes, seq_buf...)
+	bytes = append(bytes, crc_buf...)
 
-	calcCRCBytes(len_buf)
-	calcCRCBytes(cmos_buf)
-	calcCRCBytes(seq_buf)
-	if synioVerbose {
-		logger.Infof("SYNIO: CRC from synergy %04x - our calculation %04x\n", crcFromSynergy, crcHash.CRC16())
+	err = CheckSYNCRC(bytes)
+	if err == nil {
+		// errors will implicitly show  up in the log but we need to explicitly log success
+		if synioVerbose {
+			logger.Infof("SYNIO: STDUMP Success\n")
+		}
 	}
+	return
+}
 
-	if crcFromSynergy != crcHash.CRC16() {
-		err = errors.Errorf("STDUMP CRC does not match got %04x, expected %04x",
-			crcFromSynergy, crcHash.CRC16())
-		return
-	}
-	// errors will implicitly show  up in the log but we need to explicitly log success
+func CheckSYNCRC(bytes []byte) (err error) {
+	crcHash.Reset()
+	calcCRCBytes(bytes[:len(bytes)-2])
+	crc := crcHash.CRC16()
+	crcFromSYN := data.BytesToWord(bytes[len(bytes)-2], bytes[len(bytes)-1])
 	if synioVerbose {
-		logger.Infof("SYNIO: STDUMP Success\n")
+		logger.Infof("SYNIO: CRC from synergy %04x - our calculation %04x\n", crc, crcHash.CRC16())
+	}
+	if crc != crcFromSYN {
+		err = errors.Errorf("SYN CRC does not match got %04x, expected %04x", crcFromSYN, crc)
 	}
-
-	bytes = append(len_buf, cmos_buf...)
-	bytes = append(bytes, seq_buf...)
-	bytes = append(bytes, crc_buf...)
 	return
 }
diff --git a/version.go b/version.go
index 0c68221..476b987 100644
--- a/version.go
+++ b/version.go
@@ -1,3 +1,3 @@
 package main
 
-const Version = "2.6.0-beta1"
+const Version = "2.6.0-beta2"