Update canon tags

.github/ISSUE_TEMPLATE/bug_report.md

@@ -0,0 +1,28 @@
+---
+name: Bug report
+about: Create a report to help us improve
+title: ''
+labels: bug
+assignees: ''
+
+---
+
+##### **Describe the bug**
+A clear and concise description of the bug.
+
+##### **To Reproduce**
+Steps to reproduce the behavior:
+1. Provide the file with which you observed the issue (commonly named PoC)
+2. List the exact commands/functions to reproduce the issue and include any related output
+3. Mention the branch/commit in which you observed the issue (e.g., `main`, `0.27-maintenance`, `commit 9f721b40`)
+
+##### **Expected behavior**
+A short description of what you expected to happen.
+
+##### **Desktop (please complete the following information):**
+- OS and version: (e.g., Linux (Fedora 34), macOS 12, Windows 11)
+- Compiler and version: (e.g., Gcc 11.2, Clang 13.0.0, MSVC 2019)
+- Compilation mode and/or compiler flags:
+
+##### **Additional context**
+Add any other context about the problem here.

.github/ISSUE_TEMPLATE/feature_request.md

@@ -0,0 +1,20 @@
+---
+name: Feature request
+about: Suggest an idea for this project
+title: ''
+labels: request
+assignees: ''
+
+---
+
+##### **Is your feature request related to a problem?**
+A clear and concise description of the problem (e.g., I'm always frustrated when [...]; Exiv2 does not support XXX format [...])
+
+##### **Describe the solution you would like**
+A short description of what you want to happen.
+
+##### **Describe alternatives you have considered**
+A simple explanation of any alternative solutions or features you have considered.
+
+##### **Additional context**
+Add any other information about the feature request here (e.g. screenshots, URLs).

.github/codeql-queries/exiv2-code-scanning.qls

@@ -0,0 +1,3 @@
+# Reusing existing QL Pack
+- import: codeql-suites/cpp-code-scanning.qls
+  from: codeql-cpp

.github/codeql-queries/exiv2-cpp-queries/null_iterator_deref.qhelp

@@ -0,0 +1,23 @@
+<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>
+        A C++ iterator is a lot like a C pointer: if you dereference it without
+        first checking that it's valid then it can cause a crash.
+        </p>
+    </overview>
+    <recommendation>
+        <p>
+        Always check that the iterator is valid before derefencing it.
+        </p>
+    </recommendation>
+    <example>
+        <p>
+        <a href="https://github.com/Exiv2/exiv2/issues/1763">Issue 1763</a> was caused by
+        <a href="https://github.com/Exiv2/exiv2/blob/9b3ed3f9564b4ea51b43c78671435bde6b862e08/src/canonmn_int.cpp#L2755">this
+        dereference</a> of the iterator <tt>pos</tt>.
+        The bug was <a href="https://github.com/Exiv2/exiv2/pull/1767">fixed</a> by not dereferencing
+        <tt>pos</tt> if <tt>pos == metadata->end()</tt>.
+        </p>
+    </example>
+</qhelp>

.github/codeql-queries/exiv2-cpp-queries/null_iterator_deref.ql

@@ -0,0 +1,47 @@
+/**
+ * @name NULL iterator deref
+ * @description Dereferencing an iterator without checking that it's valid could cause a crash.
+ * @kind problem
+ * @problem.severity warning
+ * @id cpp/null-iterator-deref
+ * @tags security
+ *       external/cwe/cwe-476
+ */
+
+import cpp
+import semmle.code.cpp.controlflow.Guards
+import semmle.code.cpp.dataflow.DataFlow
+
+// Holds if `cond` is a condition like `use == table.end()`.
+// `eq_is_true` is `true` for `==`, `false` for '!=`.
+// Note: the `==` is actually an overloaded `operator==`.
+predicate end_condition(GuardCondition cond, Expr use, FunctionCall endCall, boolean eq_is_true) {
+  exists(FunctionCall eq |
+    exists(string opName | eq.getTarget().getName() = opName |
+      opName = "operator==" and eq_is_true = true
+      or
+      opName = "operator!=" and eq_is_true = false
+    ) and
+    DataFlow::localExprFlow(use, eq.getAnArgument()) and
+    DataFlow::localExprFlow(endCall, eq.getAnArgument()) and
+    endCall.getTarget().getName() = "end" and
+    DataFlow::localExprFlow(eq, cond)
+  )
+}
+
+from FunctionCall call, Expr use
+where
+  call.getTarget().getName() = "findKey" and
+  DataFlow::localExprFlow(call, use) and
+  use != call and
+  not use.(AssignExpr).getRValue() = call and
+  not end_condition(_, use, _, _) and
+  not exists(
+    Expr cond_use, FunctionCall endCall, GuardCondition cond, BasicBlock block, boolean branch
+  |
+    end_condition(cond, cond_use, endCall, branch) and
+    DataFlow::localExprFlow(call, cond_use) and
+    cond.controls(block, branch.booleanNot()) and
+    block.contains(use)
+  )
+select call, "Iterator returned by findKey might cause a null deref $@.", use, "here"

.github/codeql-queries/exiv2-cpp-queries/qlpack.yml

@@ -0,0 +1,4 @@
+name: exiv2-cpp-queries
+version: 0.0.0
+libraryPathDependencies: codeql-cpp
+suites: exiv2-cpp-suite

.github/codeql-queries/exiv2-cpp-queries/signed_shift.ql

@@ -0,0 +1,24 @@
+/**
+ * @name Signed shift
+ * @description Shifting a negative number is undefined behavior,
+ *              so it is risky to shift a signed number.
+ * @kind problem
+ * @problem.severity warning
+ * @id cpp/signed-shift
+ * @tags security
+ *       external/cwe/cwe-758
+ */
+
+// See the "Bitwise shift operators" section here:
+// https://en.cppreference.com/w/cpp/language/operator_arithmetic
+import cpp
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+
+from BinaryBitwiseOperation shift, Expr lhs
+where
+  (shift instanceof LShiftExpr or shift instanceof RShiftExpr) and
+  lhs = shift.getLeftOperand().getFullyConverted() and
+  lowerBound(lhs) < 0
+select shift,
+  "This signed shift could cause undefined behavior if the value is negative. Type of lhs: " +
+    lhs.getType().toString()

.github/codeql-queries/exiv2-cpp-queries/unsafe_vector_access.qhelp

@@ -0,0 +1,30 @@
+<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>
+        The <a href="https://en.cppreference.com/w/cpp/container/vector/operator_at"><tt>operator[]</tt></a> method of <a href="https://en.cppreference.com/w/cpp/container/vector"><tt>std::vector</tt></a> does not do any bounds checking on the index. It is safer to use the <a href="https://en.cppreference.com/w/cpp/container/vector/at"><tt>at()</tt></a> method, which does do bounds checking.
+        </p>
+    </overview>
+    <recommendation>
+        <p>
+        Use the <a href="https://en.cppreference.com/w/cpp/container/vector/at"><tt>at()</tt></a> method, rather than <a href="https://en.cppreference.com/w/cpp/container/vector/operator_at"><tt>operator[]</tt></a>.
+        </p>
+        <p>
+          Some uses of <a href="https://en.cppreference.com/w/cpp/container/vector/operator_at"><tt>operator[]</tt></a> are safe because they are protected by a bounds check. The query recognises the following safe coding patterns:
+        </p>
+        <ul>
+          <li><tt>if (!x.empty()) { ...x[0]... }</tt></li>
+          <li><tt>if (x.length()) { ...x[0]... }</tt></li>
+          <li><tt>if (x.size() > 2) { ...x[2]... }</tt></li>
+          <li><tt>if (x.size() == 2) { ...x[1]... }</tt></li>
+          <li><tt>if (x.size() != 0) { ...x[0]... }</tt></li>
+          <li><tt>if (i < x.size()) { ... x[i] ... }</tt></li>
+          <li><tt>if (!x.empty()) { ... x[x.size() - 1] ... }</tt></li>
+        </ul>
+    </recommendation>
+    <example>
+        <p>
+          <a href="https://github.com/Exiv2/exiv2/issues/1706">#1706</a> was caused by a lack of bounds-checking on <a href="https://github.com/Exiv2/exiv2/blob/15098f4ef50cc721ad0018218acab2ff06e60beb/include/exiv2/value.hpp#L1639">this array access</a>. The bug was <a href="https://github.com/Exiv2/exiv2/pull/1735">fixed</a> calling the <a href="https://en.cppreference.com/w/cpp/container/vector/at"><tt>at()</tt></a> method instead.
+        </p>
+    </example>
+</qhelp>

.github/codeql-queries/exiv2-cpp-queries/unsafe_vector_access.ql

@@ -0,0 +1,181 @@
+/**
+ * @name Unsafe vector access
+ * @description std::vector::operator[] does not do any runtime
+ *              bounds-checking, so it is safer to use std::vector::at()
+ * @kind problem
+ * @problem.severity warning
+ * @id cpp/unsafe-vector-access
+ * @tags security
+ *       external/cwe/cwe-125
+ */
+
+import cpp
+import semmle.code.cpp.controlflow.Guards
+import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.valuenumbering.HashCons
+
+// A call to `operator[]`.
+class ArrayIndexCall extends FunctionCall {
+  ClassTemplateInstantiation ti;
+  TemplateClass tc;
+
+  ArrayIndexCall() {
+    this.getTarget().getName() = "operator[]" and
+    ti = this.getQualifier().getType().getUnspecifiedType() and
+    tc = ti.getTemplate() and
+    tc.getSimpleName() != "map" and
+    tc.getSimpleName() != "match_results"
+  }
+
+  ClassTemplateInstantiation getClassTemplateInstantiation() { result = ti }
+
+  TemplateClass getTemplateClass() { result = tc }
+}
+
+// A call to `size` or `length`.
+class SizeCall extends FunctionCall {
+  string fname;
+
+  SizeCall() {
+    fname = this.getTarget().getName() and
+    (fname = "size" or fname = "length")
+  }
+}
+
+// `x[i]` is safe if `x` is a `std::array` (fixed-size array)
+// and `i` within the bounds of the array.
+predicate indexK_with_fixedarray(ClassTemplateInstantiation t, ArrayIndexCall call) {
+  t = call.getClassTemplateInstantiation() and
+  exists(Expr idx |
+    t.getSimpleName() = "array" and
+    idx = call.getArgument(0) and
+    lowerBound(idx) >= 0 and
+    upperBound(idx) < t.getTemplateArgument(1).(Literal).getValue().toInt()
+  )
+}
+
+// Holds if `cond` is a Boolean condition that checks the size of
+// the array. It handles the following code patterns:
+//
+// 1.  `if (!x.empty()) { ... }`
+// 2.  `if (x.length()) { ... }`
+// 3.  `if (x.size() > 2) { ... }`
+// 4.  `if (x.size() == 2) { ... }`
+// 5.  `if (x.size() != 0) { ... }`
+//
+// If it safe to assume that `x.size() >= minsize` on the exit `branch`.
+predicate minimum_size_cond(Expr cond, Expr arrayExpr, int minsize, boolean branch) {
+  // `if (!x.empty()) { ...x[0]... }`
+  exists(FunctionCall emptyCall |
+    cond = emptyCall and
+    arrayExpr = emptyCall.getQualifier() and
+    emptyCall.getTarget().getName() = "empty" and
+    minsize = 1 and
+    branch = false
+  )
+  or
+  // `if (x.length()) { ...x[0]... }`
+  exists(SizeCall sizeCall |
+    cond = sizeCall and
+    arrayExpr = sizeCall.getQualifier() and
+    minsize = 1 and
+    branch = true
+  )
+  or
+  // `if (x.size() > 2) { ...x[2]... }`
+  exists(SizeCall sizeCall, Expr k, RelationStrictness strict |
+    arrayExpr = sizeCall.getQualifier() and
+    relOpWithSwapAndNegate(cond, sizeCall, k, Greater(), strict, branch)
+  |
+    strict = Strict() and minsize = 1 + k.getValue().toInt()
+    or
+    strict = Nonstrict() and minsize = k.getValue().toInt()
+  )
+  or
+  // `if (x.size() == 2) { ...x[1]... }`
+  exists(SizeCall sizeCall, Expr k |
+    arrayExpr = sizeCall.getQualifier() and
+    eqOpWithSwapAndNegate(cond, sizeCall, k, true, branch) and
+    minsize = k.getValue().toInt()
+  )
+  or
+  // `if (x.size() != 0) { ...x[0]... }`
+  exists(SizeCall sizeCall, Expr k |
+    arrayExpr = sizeCall.getQualifier() and
+    eqOpWithSwapAndNegate(cond, sizeCall, k, false, branch) and
+    k.getValue().toInt() = 0 and
+    minsize = 1
+  )
+}
+
+// Array accesses like these are safe:
+// `if (!x.empty()) { ... x[0] ... }`
+// `if (x.size() > 2) { ... x[2] ... }`
+predicate indexK_with_check(GuardCondition guard, ArrayIndexCall call) {
+  exists(Expr arrayExpr, BasicBlock block, int i, int minsize, boolean branch |
+    minimum_size_cond(guard, arrayExpr, minsize, branch) and
+    (
+      globalValueNumber(arrayExpr) = globalValueNumber(call.getQualifier()) or
+      hashCons(arrayExpr) = hashCons(call.getQualifier())
+    ) and
+    guard.controls(block, branch) and
+    block.contains(call) and
+    i = call.getArgument(0).getValue().toInt() and
+    0 <= i and
+    i < minsize
+  )
+}
+
+// Array accesses like this are safe:
+// `if (i < x.size()) { ... x[i] ... }`
+predicate indexI_with_check(GuardCondition guard, ArrayIndexCall call) {
+  exists(Expr idx, SizeCall sizeCall, BasicBlock block, boolean branch |
+    relOpWithSwapAndNegate(guard, idx, sizeCall, Lesser(), Strict(), branch) and
+    (
+      globalValueNumber(sizeCall.getQualifier()) = globalValueNumber(call.getQualifier()) and
+      globalValueNumber(idx) = globalValueNumber(call.getArgument(0))
+      or
+      hashCons(sizeCall.getQualifier()) = hashCons(call.getQualifier()) and
+      hashCons(idx) = hashCons(call.getArgument(0))
+    ) and
+    guard.controls(block, branch) and
+    block.contains(call)
+  )
+}
+
+// Array accesses like this are safe:
+// `if (!x.empty()) { ... x[x.size() - 1] ... }`
+predicate index_last_with_check(GuardCondition guard, ArrayIndexCall call) {
+  exists(Expr arrayExpr, SubExpr minusExpr, SizeCall sizeCall, BasicBlock block, boolean branch |
+    minimum_size_cond(guard, arrayExpr, _, branch) and
+    (
+      globalValueNumber(arrayExpr) = globalValueNumber(call.getQualifier()) or
+      hashCons(arrayExpr) = hashCons(call.getQualifier())
+    ) and
+    guard.controls(block, branch) and
+    block.contains(call) and
+    minusExpr = call.getArgument(0) and
+    minusExpr.getRightOperand().getValue().toInt() = 1 and
+    DataFlow::localExprFlow(sizeCall, minusExpr.getLeftOperand()) and
+    (
+      globalValueNumber(sizeCall.getQualifier()) = globalValueNumber(call.getQualifier()) or
+      hashCons(sizeCall.getQualifier()) = hashCons(call.getQualifier())
+    )
+  )
+}
+
+from ArrayIndexCall call
+where
+  not indexK_with_fixedarray(_, call) and
+  not indexK_with_check(_, call) and
+  not indexI_with_check(_, call) and
+  not index_last_with_check(_, call) and
+  // Ignore accesses like this: `vsnprintf(&buffer[0], buffer.size(), format, args)`
+  // That's pointer arithmetic, not a deref, so it's usually a false positive.
+  not exists(AddressOfExpr addrExpr | addrExpr.getOperand() = call) and
+  // Ignore results in the xmpsdk directory.
+  not call.getLocation().getFile().getRelativePath().matches("xmpsdk/%")
+select call, "Unsafe use of operator[]. Use the at() method instead."

.github/codeql/codeql-config.yml

@@ -0,0 +1,5 @@
+name: "Exiv2 CodeQL config"
+
+queries:
+    - uses: ./.github/codeql-queries/exiv2-code-scanning.qls
+    - uses: ./.github/codeql-queries/exiv2-cpp-queries