Repository: zmap/zlint
Pull request #276 (2019-03-30 14:46:39)

lints: fix bug in lint_qcstatem_qclimitvalue_valid.go
The `e_qcstatem_qclimitvalue_valid` linter added in https://github.com/zmap/zlint/commit/0f862af0a6db1b6bf2f7fffef5cb160583b93537 was blindly performing a type cast that could fail. This fix checks if the type cast fails and returns an error lint result instead of panicing.

A small test cases is added that lints the certificate that triggered #274. Before applying the fix in this branch the test fails as expected:
```
$ go test -v --test.run TestQcStatemQcLimitValueValid ./lints/...
=== RUN   TestQcStatemQcLimitValueValid
--- FAIL: TestQcStatemQcLimitValueValid (0.00s)
panic: interface conversion: util.EtsiQcStmtIf is util.EtsiQcSscd, not util.EtsiQcLimitValue [recovered]
	panic: interface conversion: util.EtsiQcStmtIf is util.EtsiQcSscd, not util.EtsiQcLimitValue

goroutine 5 [running]:
testing.tRunner.func1(0xc00012a600)
	/usr/lib/go/src/testing/testing.go:830 +0x388
panic(0x7c9040, 0xc0002dea80)
	/usr/lib/go/src/runtime/panic.go:522 +0x1b5
github.com/zmap/zlint/lints.(*qcStatemQcLimitValueValid).Execute(0xc3f450, 0xc000272a80, 0xc0002b0101)
	/home/daniel/go/src/github.com/zmap/zlint/lints/lint_qcstatem_qclimitvalue_valid.go:60 +0x59a
github.com/zmap/zlint/lints.(*Lint).Execute(0xc0002aac00, 0xc000272a80, 0x85b992)
	/home/daniel/go/src/github.com/zmap/zlint/lints/base.go:114 +0x94
github.com/zmap/zlint/lints.TestQcStatemQcLimitValueValid(0xc00012a600)
	/home/daniel/go/src/github.com/zmap/zlint/lints/lint_qcstatem_qclimitvalue_valid_test.go:25 +0x1e8
testing.tRunner(0xc00012a600, 0x878ef0)
	/usr/lib/go/src/testing/testing.go:865 +0xc0
created by testing.(*T).Run
	/usr/lib/go/src/testing/testing.go:916 +0x357
FAIL	github.com/zmap/zlint/lints	0.008s
```

Afterwards, it passes :tada:

```
$ go test -v --test.run TestQcStatemQcLimitValueValid ./lints/...
=== RUN   TestQcStatemQcLimitValueValid
--- PASS: TestQcStatemQcLimitValueValid (0.00s)
PASS
ok  	github.com/zmap/zlint/lints	0.005s
```

**Note to reviewers:** I'm extremely unfamiliar with **all** of these ETSI specifications. 0f862af0a6db1b6bf2f7fffef5cb160583b93537 didn't include any unit tests for  `lint_qcstatem_qclimitvalue_valid.go` and I'm largely guessing that if this cast fails it should produce a failing lint result. Unfortunately if improvements are needed beyond fixing the immediate crasher bug I'm not the right person to do them.

Resolves https://github.com/zmap/zlint/issues/274


Patch:
diff --git a/lints/lint_qcstatem_qclimitvalue_valid.go b/lints/lint_qcstatem_qclimitvalue_valid.go
index 8e0b2672..4dac4229 100644
--- a/lints/lint_qcstatem_qclimitvalue_valid.go
+++ b/lints/lint_qcstatem_qclimitvalue_valid.go
@@ -58,7 +58,10 @@ func (l *qcStatemQcLimitValueValid) Execute(c *x509.Certificate) *LintResult {
 	s := util.ParseQcStatem(ext.Value, *l.getStatementOid())
 	errString += s.GetErrorInfo()
 	if len(errString) == 0 {
-		qcLv := s.(util.EtsiQcLimitValue)
+		qcLv, ok := s.(util.EtsiQcLimitValue)
+		if !ok {
+			return &LintResult{Status: Error, Details: "parsed QcStatem is not a EtsiQcLimitValue"}
+		}
 		if qcLv.Amount < 0 {
 			util.AppendToStringSemicolonDelim(&errString, "amount is negative")
 		}
@@ -84,13 +87,13 @@ func (l *qcStatemQcLimitValueValid) Execute(c *x509.Certificate) *LintResult {
 	}
 }
 
-//func init() {
-//	RegisterLint(&Lint{
-//		Name:          "e_qcstatem_qclimitvalue_valid",
-//		Description:   "Checks that a QC Statement of the type id-etsi-qcs-QcLimitValue has the correct form",
-//		Citation:      "ETSI EN 319 412 - 5 V2.2.1 (2017 - 11) / Section 4.3.2",
-//		Source:        EtsiEsi,
-//		EffectiveDate: util.EtsiEn319_412_5_V2_2_1_Date,
-//		Lint:          &qcStatemQcLimitValueValid{},
-//	})
-//}
+func init() {
+	RegisterLint(&Lint{
+		Name:          "e_qcstatem_qclimitvalue_valid",
+		Description:   "Checks that a QC Statement of the type id-etsi-qcs-QcLimitValue has the correct form",
+		Citation:      "ETSI EN 319 412 - 5 V2.2.1 (2017 - 11) / Section 4.3.2",
+		Source:        EtsiEsi,
+		EffectiveDate: util.EtsiEn319_412_5_V2_2_1_Date,
+		Lint:          &qcStatemQcLimitValueValid{},
+	})
+}


Test patch:
diff --git a/lints/lint_qcstatem_qclimitvalue_valid_test.go b/lints/lint_qcstatem_qclimitvalue_valid_test.go
new file mode 100644
index 00000000..5f8721ab
--- /dev/null
+++ b/lints/lint_qcstatem_qclimitvalue_valid_test.go
@@ -0,0 +1,33 @@
+/*
+ * ZLint Copyright 2018 Regents of the University of Michigan
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy
+ * of the License at http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package lints
+
+import (
+	"testing"
+)
+
+func TestQcStatemQcLimitValueValid(t *testing.T) {
+	m := map[string]LintStatus{
+		"QcStmtInvalidLimitValue.pem": Error,
+	}
+	for inputPath, expected := range m {
+		inputPath = "../testlint/testCerts/" + inputPath
+		out := Lints["e_qcstatem_qclimitvalue_valid"].Execute(ReadCertificate(inputPath))
+
+		if out.Status != expected {
+			t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status)
+		}
+	}
+}
diff --git a/testlint/testCerts/QcStmtInvalidLimitValue.pem b/testlint/testCerts/QcStmtInvalidLimitValue.pem
new file mode 100644
index 00000000..53ee3a57
--- /dev/null
+++ b/testlint/testCerts/QcStmtInvalidLimitValue.pem
@@ -0,0 +1,45 @@
+-----BEGIN CERTIFICATE-----
+MIIH9DCCBdygAwIBAgIUSdoOEheYrV3jVyQ92o9dT4pmkW4wDQYJKoZIhvcNAQEL
+BQAwgYMxCzAJBgNVBAYTAk5MMSAwHgYDVQQKDBdRdW9WYWRpcyBUcnVzdGxpbmsg
+Qi5WLjEXMBUGA1UEYQwOTlRSTkwtMzAyMzc0NTkxOTA3BgNVBAMMMFF1b1ZhZGlz
+IFBLSW92ZXJoZWlkIE9yZ2FuaXNhdGllIFBlcnNvb24gQ0EgLSBHMzAeFw0xODAy
+MDgxNTMxNTNaFw0yMTAyMDgxNTQxMDBaMIGOMQswCQYDVQQGEwJOTDEPMA0GA1UE
+CgwGTG9naXVzMRgwFgYDVQQFEw82MDYxNDUzODE5Mjc0OTAxFjAUBgNVBCoMDUpv
+Y2hlbSBKZXJvZW4xFjAUBgNVBAQMDXZhbiBkZW4gQmVyZ2UxJDAiBgNVBAMMG0pv
+Y2hlbSBKZXJvZW4gdmFuIGRlbiBCZXJnZTCCASIwDQYJKoZIhvcNAQEBBQADggEP
+ADCCAQoCggEBAMCCqxzQ3LcACWCsk4RCeEJE/KFthLSkd7ZDLgqqX2ckQ1QnlDky
+7BNdI22zaw2bgvKCeEPx2R52AAy0aYIpW3XMkCUEWc0YlZPL8v+v/7Un9qi4QQrR
+Xim01dcZ72PGWasG8OQoYeq42uNMyvUP306RYNGCqx2Qt4YImzN3Eb/Eag7iwqvb
+LWZaxHPqxa3MpklOEH+MQTPoJAIq2OmPcU/viI5C5Zgb8GjatD9cLOKb9dYFtxCl
+1k0xVcwC7AwfOCxQXuKUHkDf19WMfkazKfRN5wXMY3/jEmFhhFPN3virlaVuZPg1
+jMDvM6FAf8J+xek9XUoEWQe9mU4MEZxgL6MCAwEAAaOCA1EwggNNMHsGCCsGAQUF
+BwEBBG8wbTA8BggrBgEFBQcwAoYwaHR0cDovL3RydXN0LnF1b3ZhZGlzZ2xvYmFs
+LmNvbS9wa2lvcGVyc29uZzMuY3J0MC0GCCsGAQUFBzABhiFodHRwOi8vdXcub2Nz
+cC5xdW92YWRpc2dsb2JhbC5jb20wHQYDVR0OBBYEFK9HG9cwfvKhL/DA1GDRFL4j
+N7FCMB8GA1UdIwQYMBaAFFGoa/eCJJToi6O2KTuOtrKdKPmLMIIBMgYDVR0gBIIB
+KTCCASUwggEhBgpghBABh2sBAgUCMIIBETA0BggrBgEFBQcCARYoaHR0cDovL3d3
+dy5xdW92YWRpc2dsb2JhbC5jb20vcmVwb3NpdG9yeTCB2AYIKwYBBQUHAgIwgcsM
+gchSZWxpYW5jZSBvbiB0aGlzIGNlcnRpZmljYXRlIGJ5IGFueSBwYXJ0eSBhc3N1
+bWVzIGFjY2VwdGFuY2Ugb2YgdGhlIHJlbGV2YW50IFF1b1ZhZGlzIENlcnRpZmlj
+YXRpb24gUHJhY3RpY2UgU3RhdGVtZW50IGFuZCBvdGhlciBkb2N1bWVudHMgaW4g
+dGhlIFF1b1ZhZGlzIHJlcG9zaXRvcnkgIChodHRwOi8vd3d3LnF1b3ZhZGlzZ2xv
+YmFsLmNvbSApLjA/BgNVHR8EODA2MDSgMqAwhi5odHRwOi8vY3JsLnF1b3ZhZGlz
+Z2xvYmFsLmNvbS9wa2lvcGVyc29uZzMuY3JsMA4GA1UdDwEB/wQEAwIGQDAfBgNV
+HSUEGDAWBggrBgEFBQcDBAYKKwYBBAGCNwoDDDBjBgNVHREEXDBagR1qb2NoZW0u
+dmFuZGVuLmJlcmdlQGxvZ2l1cy5ubKA5BgorBgEEAYI3FAIDoCsMKTYwNjE0NTM4
+MTkyNzQ5MEAyLjE2LjUyOC4xLjEwMDMuMS4zLjUuMi4xMIGABggrBgEFBQcBAwR0
+MHIwCgYIKwYBBQUHCwIwCAYGBACORgEBMAgGBgQAjkYBBDATBgYEAI5GAQYwCQYH
+BACORgEGATA7BgYEAI5GAQUwMTAvFilodHRwczovL3d3dy5xdW92YWRpc2dsb2Jh
+bC5jb20vcmVwb3NpdG9yeRMCZW4wDQYJKoZIhvcNAQELBQADggIBAHppQw6aqid1
+IcOAIR9aEUXhFLwIP4FGrS4dwTgruo5OffaI1nMNIKCw4oHjWWUJ9bzfHn1RK4JI
+sgRCEjAFd7AP/PaDe0tX5CBbeiSmauKFhC6SfnX0uTDIZz5hpLvUOI8RIk1xlXf/
+hg9UtGBExNnTnjY9HomEkHvaCtp0HmmV4KsjOcq/Avl+diKpVYW3yaZiXJ7naGXg
+I0/9CBQoFCac2a9rMOg6ZK44hrRvq3G12LR+9STj+dien5+Jr2ZLmwG1XpdEC5Cv
+Mr49iSoSe0ndqRuHIVpL9dW3MLZnT/XzWMYfMchtA0jKhDTloH18JOtiv60giiDT
+ZNOXjPm/MUqSpL2wyvk+pX3pI6OasU3TTdgoV3A8pFnSltvKGvqJfN4xK66r71UI
+V6fF9znGzCDDANA3HDS/IGWh5sgxFtH3b4ZA8bjS1wnMJG8Zp6Q1nKutQuV3Nf48
+kxcQhSjOgs2QE2pGfGmnEwM9isXqx1r3O4XhmBj91/VJOA1i5dM+VCAFyEluApoq
+KKkUSiuPCbhsXuF+JBfcps/XH5G+GtHvhaj8bJqizcrbjOqBgCh6Q6MLschHjQiI
+9P1EcY/BoPSS8nr26rvappNvA5K5zDqRtqJBepwVE3D3j/oO4CSGfEupEpSp8btn
+j6UXGD1EDP1UPIii1W4rfoTXAB8IfkSq
+-----END CERTIFICATE-----