schemas: escape the delimiter inside a quoted identifier

quoteWordTo wrapped an identifier in the dialect delimiter but wrote the body
verbatim, so a delimiter inside the identifier closed the quote the writer
opened and the remainder ran as SQL. Desc/Asc route caller strings straight
here through Quoter.QuoteTo:

  Desc("name`,(select token from secret)--")
    -> ORDER BY `name`,(select token from secret)--` DESC

two quoted identifiers plus a bare subquery, not one identifier. The word looks
quoted, so callers treat Desc/Asc as identifier-safe; it was not.

Double the closing delimiter — the standard SQL identifier escape — and force a
quoted form for any word that carries one, even a dialect that would otherwise
emit it bare. A name that smuggled a delimiter becomes one identifier that does
not exist, so it fails closed:

  -> ORDER BY `name``,(select token from secret)--` DESC   (no such column)

Same for the " and ] delimiters. Legitimate names hold no delimiter and are
unchanged, so the existing quoter cases and the round-trip through Trim still
hold. Tests assert the emitted quoted string per dialect.

Co-authored-by: Hanzo Dev <[email protected]>

schemas/quote.go

@@ -148,22 +148,49 @@ func (q Quoter) quoteWordTo(buf *strings.Builder, word string) error {
 		return err
 	}
 
+	// A closing delimiter inside the body would end the quote opened below and
+	// turn the rest of the word into executable SQL — Desc("x`,(subquery)--")
+	// otherwise emits `x`,(subquery)--`, two identifiers plus bare SQL. Force a
+	// quoted form for any word that carries a delimiter, even a dialect that
+	// would otherwise emit it bare, and double the delimiter (the standard SQL
+	// identifier escape) so the body cannot break out. A name that smuggled a
+	// delimiter becomes one identifier that does not exist: it fails closed.
+	carriesDelim := strings.IndexByte(realWord, q.Suffix) >= 0 ||
+		strings.IndexByte(realWord, q.Prefix) >= 0
+
 	isReserved := q.IsReserved(realWord)
-	if isReserved && realWord != "*" {
+	if (isReserved || carriesDelim) && realWord != "*" {
 		if err := buf.WriteByte(q.Prefix); err != nil {
 			return err
 		}
+		if err := writeEscapedIdent(buf, realWord, q.Suffix); err != nil {
+			return err
+		}
+		return buf.WriteByte(q.Suffix)
 	}
 	if _, err := buf.WriteString(realWord); err != nil {
 		return err
 	}
-	if isReserved && realWord != "*" {
-		return buf.WriteByte(q.Suffix)
-	}
 
 	return nil
 }
 
+// writeEscapedIdent writes the body of a quoted identifier, doubling every
+// occurrence of the closing delimiter so it cannot terminate the quote.
+func writeEscapedIdent(buf *strings.Builder, word string, suffix byte) error {
+	for i := 0; i < len(word); i++ {
+		if word[i] == suffix {
+			if err := buf.WriteByte(suffix); err != nil {
+				return err
+			}
+		}
+		if err := buf.WriteByte(word[i]); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // QuoteTo quotes the table or column names. i.e. if the quotes are [ and ]
 //
 //	name -> [name]

schemas/quote_injection_test.go

@@ -0,0 +1,63 @@
+// Copyright 2024 The Xorm Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package schemas
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// quoteWordTo wraps an identifier in the dialect delimiter but wrote the body
+// verbatim, so a delimiter INSIDE the identifier closed the quote the writer
+// opened and the remainder executed. Desc("name`,(subquery)--") became
+//
+//	`name`,(subquery)--`
+//
+// two quoted identifiers plus bare SQL, not one identifier. The escape is to
+// DOUBLE the closing delimiter (standard SQL identifier escaping); this asserts
+// the emitted string, so it fails if the escape is dropped.
+func TestQuoteWordCannotBreakOutOfItsDelimiter(t *testing.T) {
+	cases := []struct {
+		name   string
+		quoter Quoter
+		in     string
+		want   string
+	}{
+		{
+			name:   "backtick (mysql/sqlite)",
+			quoter: Quoter{'`', '`', AlwaysReserve},
+			in:     "name`,(select/**/token/**/from/**/secret)--",
+			want:   "`name``,(select/**/token/**/from/**/secret)--`",
+		},
+		{
+			name:   "double-quote (postgres/standard)",
+			quoter: Quoter{'"', '"', AlwaysReserve},
+			in:     `name",(select/**/token/**/from/**/secret)--`,
+			want:   `"name"",(select/**/token/**/from/**/secret)--"`,
+		},
+		{
+			name:   "bracket (mssql) — only the closing ] is doubled",
+			quoter: Quoter{'[', ']', AlwaysReserve},
+			in:     `name],(select/**/token/**/from/**/secret)--`,
+			want:   `[name]],(select/**/token/**/from/**/secret)--]`,
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			got := c.quoter.Quote(c.in)
+			t.Logf("Quote(%q) = %q", c.in, got)
+			assert.Equal(t, c.want, got)
+
+			// Structural invariant: strip the outer delimiter pair; every closing
+			// delimiter left in the body must be doubled, so its count is even.
+			// A lone one would terminate the quote and start bare SQL.
+			inner := got[1 : len(got)-1]
+			assert.Equal(t, 0, strings.Count(inner, string(c.quoter.Suffix))%2,
+				"interior closing delimiters must all be escaped (doubled)")
+		})
+	}
+}