Skip to content

Commit bfb7b1d

Browse files
CopilotCopilot
authored
Commit 6: Fix POSIX-bracket mis-parses revealed by Commit 5
POSIX bracket expressions ([:name:], [.x.], [=x=]) are only valid nested inside another character class in std::regex. The three predicates now additionally require that at position `start` we are inside an outer character class, approximated by requiring more non-escaped `[` than non-escaped `]` before `start`. A well-formed POSIX bracket contributes one `[` and one `]` at/after `start`, so the check is unaffected by earlier POSIX brackets. Also stop emitting individual charSetTokens for characters inside a POSIX bracket (`inAnyPosixBracket` guard), which removes the spurious `[RegExpCharacterRange] :]-z` on [[:alpha:]-z]. Post-fix, unnested `[:digit:]`, `[:alpha:]`, and `a[:b:]c` are correctly parsed as ordinary character classes / literal sequences rather than `RegExpNamedCharacterProperty`; and the [[:alpha:]-z] range no longer crosses the POSIX bracket boundary. Bundle: github/codeql-action codeql-bundle-v2.26.1 (CodeQL CLI 2.26.1).
1 parent 8d53e36 commit bfb7b1d

4 files changed

Lines changed: 176 additions & 20 deletions

File tree

‎cpp/ql/lib/semmle/code/cpp/regex/internal/ParseRegExp.qll‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ class RegExp extends StringLiteral {
162162
or
163163
this.namedCharacterProperty(start, end, _)
164164
or
165-
exists(this.nonEscapedCharAt(start)) and end = start + 1
165+
exists(this.nonEscapedCharAt(start)) and
166+
not this.inAnyPosixBracket(start) and
167+
end = start + 1
166168
)
167169
or
168170
this.charSetToken(charsetStart, _, start) and
@@ -172,6 +174,7 @@ class RegExp extends StringLiteral {
172174
this.namedCharacterProperty(start, end, _)
173175
or
174176
exists(this.nonEscapedCharAt(start)) and
177+
not this.inAnyPosixBracket(start) and
175178
end = start + 1 and
176179
not this.getChar(start) = "]"
177180
)
@@ -313,6 +316,13 @@ class RegExp extends StringLiteral {
313316
private predicate posixStyleNamedCharacterProperty(int start, int end, string name) {
314317
this.getChar(start) = "[" and
315318
this.getChar(start + 1) = ":" and
319+
// POSIX bracket expressions are only valid nested inside another character
320+
// class. Approximate this by requiring at least one more (non-escaped)
321+
// opening `[` than closing `]` before `start`. A well-formed POSIX bracket
322+
// contributes one `[` and one `]` at/after `start`, so this check is
323+
// unaffected by other POSIX brackets earlier in the text.
324+
count(int p | p < start and this.nonEscapedCharAt(p) = "[") >
325+
count(int p | p < start and this.nonEscapedCharAt(p) = "]") and
316326
end =
317327
min(int e |
318328
e > start and
@@ -337,6 +347,8 @@ class RegExp extends StringLiteral {
337347
private predicate posixCollatingSymbol(int start, int end, string name) {
338348
this.getChar(start) = "[" and
339349
this.getChar(start + 1) = "." and
350+
count(int p | p < start and this.nonEscapedCharAt(p) = "[") >
351+
count(int p | p < start and this.nonEscapedCharAt(p) = "]") and
340352
end =
341353
min(int e |
342354
e > start and
@@ -355,6 +367,8 @@ class RegExp extends StringLiteral {
355367
private predicate posixEquivalenceClass(int start, int end, string name) {
356368
this.getChar(start) = "[" and
357369
this.getChar(start + 1) = "=" and
370+
count(int p | p < start and this.nonEscapedCharAt(p) = "[") >
371+
count(int p | p < start and this.nonEscapedCharAt(p) = "]") and
358372
end =
359373
min(int e |
360374
e > start and

‎cpp/ql/test/library-tests/regex/locations.expected‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,19 @@
66
| test.cpp:143:29:143:29 | p | 23 | 5 | 6 | 29 | 29 |
77
| test.cpp:143:30:143:30 | h | 23 | 6 | 7 | 30 | 30 |
88
| test.cpp:143:31:143:31 | a | 23 | 7 | 8 | 31 | 31 |
9+
| test.cpp:146:24:146:35 | []a[:alpha:] | 23 | 0 | 12 | 24 | 35 |
910
| test.cpp:146:24:146:36 | []a[:alpha:]] | 23 | 0 | 13 | 24 | 36 |
1011
| test.cpp:146:25:146:25 | ] | 23 | 1 | 2 | 25 | 25 |
1112
| test.cpp:146:26:146:26 | a | 23 | 2 | 3 | 26 | 26 |
12-
| test.cpp:146:27:146:35 | [:alpha:] | 23 | 3 | 12 | 27 | 35 |
13+
| test.cpp:146:27:146:27 | [ | 23 | 3 | 4 | 27 | 27 |
14+
| test.cpp:146:28:146:28 | : | 23 | 4 | 5 | 28 | 28 |
15+
| test.cpp:146:29:146:29 | a | 23 | 5 | 6 | 29 | 29 |
16+
| test.cpp:146:30:146:30 | l | 23 | 6 | 7 | 30 | 30 |
17+
| test.cpp:146:31:146:31 | p | 23 | 7 | 8 | 31 | 31 |
18+
| test.cpp:146:32:146:32 | h | 23 | 8 | 9 | 32 | 32 |
19+
| test.cpp:146:33:146:33 | a | 23 | 9 | 10 | 33 | 33 |
20+
| test.cpp:146:34:146:34 | : | 23 | 10 | 11 | 34 | 34 |
21+
| test.cpp:146:36:146:36 | ] | 23 | 12 | 13 | 36 | 36 |
1322
| test.cpp:149:25:149:53 | [[:alpha:][:digit:][:space:]] | 24 | 0 | 29 | 25 | 53 |
1423
| test.cpp:149:26:149:34 | [:alpha:] | 24 | 1 | 10 | 26 | 34 |
1524
| test.cpp:149:35:149:43 | [:digit:] | 24 | 10 | 19 | 35 | 43 |

‎cpp/ql/test/library-tests/regex/parse.expected‎

Lines changed: 92 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -515,26 +515,81 @@ test.cpp:
515515

516516
# 128| [RegExpConstant, RegExpNormalChar] f
517517

518-
# 131| [RegExpNamedCharacterProperty] [:digit:]
518+
# 131| [RegExpCharacterClass] [:digit:]
519+
#-----| 0 -> [RegExpConstant, RegExpNormalChar] :
520+
#-----| 1 -> [RegExpConstant, RegExpNormalChar] d
521+
#-----| 2 -> [RegExpConstant, RegExpNormalChar] i
522+
#-----| 3 -> [RegExpConstant, RegExpNormalChar] g
523+
#-----| 4 -> [RegExpConstant, RegExpNormalChar] i
524+
#-----| 5 -> [RegExpConstant, RegExpNormalChar] t
525+
#-----| 6 -> [RegExpConstant, RegExpNormalChar] :
526+
527+
# 131| [RegExpConstant, RegExpNormalChar] :
528+
529+
# 131| [RegExpConstant, RegExpNormalChar] d
530+
531+
# 131| [RegExpConstant, RegExpNormalChar] i
532+
533+
# 131| [RegExpConstant, RegExpNormalChar] g
534+
535+
# 131| [RegExpConstant, RegExpNormalChar] i
536+
537+
# 131| [RegExpConstant, RegExpNormalChar] t
538+
539+
# 131| [RegExpConstant, RegExpNormalChar] :
540+
541+
# 134| [RegExpCharacterClass] [:alpha:]
542+
#-----| 0 -> [RegExpConstant, RegExpNormalChar] :
543+
#-----| 1 -> [RegExpConstant, RegExpNormalChar] a
544+
#-----| 2 -> [RegExpConstant, RegExpNormalChar] l
545+
#-----| 3 -> [RegExpConstant, RegExpNormalChar] p
546+
#-----| 4 -> [RegExpConstant, RegExpNormalChar] h
547+
#-----| 5 -> [RegExpConstant, RegExpNormalChar] a
548+
#-----| 6 -> [RegExpConstant, RegExpNormalChar] :
549+
550+
# 134| [RegExpConstant, RegExpNormalChar] :
551+
552+
# 134| [RegExpConstant, RegExpNormalChar] a
553+
554+
# 134| [RegExpConstant, RegExpNormalChar] l
555+
556+
# 134| [RegExpConstant, RegExpNormalChar] p
519557

520-
# 134| [RegExpNamedCharacterProperty] [:alpha:]
558+
# 134| [RegExpConstant, RegExpNormalChar] h
559+
560+
# 134| [RegExpConstant, RegExpNormalChar] a
561+
562+
# 134| [RegExpConstant, RegExpNormalChar] :
521563

522564
# 137| [RegExpConstant, RegExpNormalChar] a
523565

524-
# 137| [RegExpNamedCharacterProperty] [:b:]
566+
# 137| [RegExpSequence] a[:b:]c
567+
#-----| 0 -> [RegExpConstant, RegExpNormalChar] a
568+
#-----| 1 -> [RegExpCharacterClass] [:b:]
569+
#-----| 2 -> [RegExpConstant, RegExpNormalChar] c
570+
571+
# 137| [RegExpCharacterClass] [:b:]
572+
#-----| 0 -> [RegExpConstant, RegExpNormalChar] :
573+
#-----| 1 -> [RegExpConstant, RegExpNormalChar] b
574+
#-----| 2 -> [RegExpConstant, RegExpNormalChar] :
575+
576+
# 137| [RegExpConstant, RegExpNormalChar] :
577+
578+
# 137| [RegExpConstant, RegExpNormalChar] b
579+
580+
# 137| [RegExpConstant, RegExpNormalChar] :
525581

526582
# 137| [RegExpConstant, RegExpNormalChar] c
527583

528584
# 140| [RegExpCharacterClass] [[:alpha:]-z]
529-
#-----| 0 -> [RegExpNamedCharacterProperty] [:alpha:]
585+
#-----| 0 -> [RegExpCharacterRange] [:alpha:]-z
530586

531587
# 140| [RegExpNamedCharacterProperty] [:alpha:]
532588

533-
# 140| [RegExpCharacterRange] :]-z
589+
# 140| [RegExpCharacterRange] [:alpha:]-z
590+
#-----| 0 -> [RegExpNamedCharacterProperty] [:alpha:]
534591
#-----| 1 -> [RegExpConstant, RegExpNormalChar] z
535592

536-
# 140| [RegExpConstant, RegExpNormalChar] -
537-
538593
# 140| [RegExpConstant, RegExpNormalChar] z
539594

540595
# 143| [RegExpCharacterClass] [[:alpha]
@@ -560,16 +615,43 @@ test.cpp:
560615

561616
# 143| [RegExpConstant, RegExpNormalChar] a
562617

563-
# 146| [RegExpCharacterClass] []a[:alpha:]]
618+
# 146| [RegExpCharacterClass] []a[:alpha:]
564619
#-----| 0 -> [RegExpConstant, RegExpNormalChar] ]
565620
#-----| 1 -> [RegExpConstant, RegExpNormalChar] a
566-
#-----| 2 -> [RegExpNamedCharacterProperty] [:alpha:]
621+
#-----| 2 -> [RegExpConstant, RegExpNormalChar] [
622+
#-----| 3 -> [RegExpConstant, RegExpNormalChar] :
623+
#-----| 4 -> [RegExpConstant, RegExpNormalChar] a
624+
#-----| 5 -> [RegExpConstant, RegExpNormalChar] l
625+
#-----| 6 -> [RegExpConstant, RegExpNormalChar] p
626+
#-----| 7 -> [RegExpConstant, RegExpNormalChar] h
627+
#-----| 8 -> [RegExpConstant, RegExpNormalChar] a
628+
#-----| 9 -> [RegExpConstant, RegExpNormalChar] :
629+
630+
# 146| [RegExpSequence] []a[:alpha:]]
631+
#-----| 0 -> [RegExpCharacterClass] []a[:alpha:]
632+
#-----| 1 -> [RegExpConstant, RegExpNormalChar] ]
567633

568634
# 146| [RegExpConstant, RegExpNormalChar] ]
569635

570636
# 146| [RegExpConstant, RegExpNormalChar] a
571637

572-
# 146| [RegExpNamedCharacterProperty] [:alpha:]
638+
# 146| [RegExpConstant, RegExpNormalChar] [
639+
640+
# 146| [RegExpConstant, RegExpNormalChar] :
641+
642+
# 146| [RegExpConstant, RegExpNormalChar] a
643+
644+
# 146| [RegExpConstant, RegExpNormalChar] l
645+
646+
# 146| [RegExpConstant, RegExpNormalChar] p
647+
648+
# 146| [RegExpConstant, RegExpNormalChar] h
649+
650+
# 146| [RegExpConstant, RegExpNormalChar] a
651+
652+
# 146| [RegExpConstant, RegExpNormalChar] :
653+
654+
# 146| [RegExpConstant, RegExpNormalChar] ]
573655

574656
# 149| [RegExpCharacterClass] [[:alpha:][:digit:][:space:]]
575657
#-----| 0 -> [RegExpNamedCharacterProperty] [:alpha:]

‎cpp/ql/test/library-tests/regex/regexp.expected‎

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -200,15 +200,32 @@ term
200200
| test.cpp:128:37:128:37 | a | RegExpConstant,RegExpNormalChar |
201201
| test.cpp:128:37:128:39 | a-f | RegExpCharacterRange |
202202
| test.cpp:128:39:128:39 | f | RegExpConstant,RegExpNormalChar |
203-
| test.cpp:131:24:131:32 | [:digit:] | RegExpNamedCharacterProperty |
204-
| test.cpp:134:24:134:32 | [:alpha:] | RegExpNamedCharacterProperty |
203+
| test.cpp:131:24:131:32 | [:digit:] | RegExpCharacterClass |
204+
| test.cpp:131:25:131:25 | : | RegExpConstant,RegExpNormalChar |
205+
| test.cpp:131:26:131:26 | d | RegExpConstant,RegExpNormalChar |
206+
| test.cpp:131:27:131:27 | i | RegExpConstant,RegExpNormalChar |
207+
| test.cpp:131:28:131:28 | g | RegExpConstant,RegExpNormalChar |
208+
| test.cpp:131:29:131:29 | i | RegExpConstant,RegExpNormalChar |
209+
| test.cpp:131:30:131:30 | t | RegExpConstant,RegExpNormalChar |
210+
| test.cpp:131:31:131:31 | : | RegExpConstant,RegExpNormalChar |
211+
| test.cpp:134:24:134:32 | [:alpha:] | RegExpCharacterClass |
212+
| test.cpp:134:25:134:25 | : | RegExpConstant,RegExpNormalChar |
213+
| test.cpp:134:26:134:26 | a | RegExpConstant,RegExpNormalChar |
214+
| test.cpp:134:27:134:27 | l | RegExpConstant,RegExpNormalChar |
215+
| test.cpp:134:28:134:28 | p | RegExpConstant,RegExpNormalChar |
216+
| test.cpp:134:29:134:29 | h | RegExpConstant,RegExpNormalChar |
217+
| test.cpp:134:30:134:30 | a | RegExpConstant,RegExpNormalChar |
218+
| test.cpp:134:31:134:31 | : | RegExpConstant,RegExpNormalChar |
205219
| test.cpp:137:24:137:24 | a | RegExpConstant,RegExpNormalChar |
206-
| test.cpp:137:25:137:29 | [:b:] | RegExpNamedCharacterProperty |
220+
| test.cpp:137:24:137:30 | a[:b:]c | RegExpSequence |
221+
| test.cpp:137:25:137:29 | [:b:] | RegExpCharacterClass |
222+
| test.cpp:137:26:137:26 | : | RegExpConstant,RegExpNormalChar |
223+
| test.cpp:137:27:137:27 | b | RegExpConstant,RegExpNormalChar |
224+
| test.cpp:137:28:137:28 | : | RegExpConstant,RegExpNormalChar |
207225
| test.cpp:137:30:137:30 | c | RegExpConstant,RegExpNormalChar |
208226
| test.cpp:140:24:140:36 | [[:alpha:]-z] | RegExpCharacterClass |
209227
| test.cpp:140:25:140:33 | [:alpha:] | RegExpNamedCharacterProperty |
210-
| test.cpp:140:32:140:35 | :]-z | RegExpCharacterRange |
211-
| test.cpp:140:34:140:34 | - | RegExpConstant,RegExpNormalChar |
228+
| test.cpp:140:25:140:35 | [:alpha:]-z | RegExpCharacterRange |
212229
| test.cpp:140:35:140:35 | z | RegExpConstant,RegExpNormalChar |
213230
| test.cpp:143:24:143:32 | [[:alpha] | RegExpCharacterClass |
214231
| test.cpp:143:25:143:25 | [ | RegExpConstant,RegExpNormalChar |
@@ -218,10 +235,19 @@ term
218235
| test.cpp:143:29:143:29 | p | RegExpConstant,RegExpNormalChar |
219236
| test.cpp:143:30:143:30 | h | RegExpConstant,RegExpNormalChar |
220237
| test.cpp:143:31:143:31 | a | RegExpConstant,RegExpNormalChar |
221-
| test.cpp:146:24:146:36 | []a[:alpha:]] | RegExpCharacterClass |
238+
| test.cpp:146:24:146:35 | []a[:alpha:] | RegExpCharacterClass |
239+
| test.cpp:146:24:146:36 | []a[:alpha:]] | RegExpSequence |
222240
| test.cpp:146:25:146:25 | ] | RegExpConstant,RegExpNormalChar |
223241
| test.cpp:146:26:146:26 | a | RegExpConstant,RegExpNormalChar |
224-
| test.cpp:146:27:146:35 | [:alpha:] | RegExpNamedCharacterProperty |
242+
| test.cpp:146:27:146:27 | [ | RegExpConstant,RegExpNormalChar |
243+
| test.cpp:146:28:146:28 | : | RegExpConstant,RegExpNormalChar |
244+
| test.cpp:146:29:146:29 | a | RegExpConstant,RegExpNormalChar |
245+
| test.cpp:146:30:146:30 | l | RegExpConstant,RegExpNormalChar |
246+
| test.cpp:146:31:146:31 | p | RegExpConstant,RegExpNormalChar |
247+
| test.cpp:146:32:146:32 | h | RegExpConstant,RegExpNormalChar |
248+
| test.cpp:146:33:146:33 | a | RegExpConstant,RegExpNormalChar |
249+
| test.cpp:146:34:146:34 | : | RegExpConstant,RegExpNormalChar |
250+
| test.cpp:146:36:146:36 | ] | RegExpConstant,RegExpNormalChar |
225251
| test.cpp:149:25:149:53 | [[:alpha:][:digit:][:space:]] | RegExpCharacterClass |
226252
| test.cpp:149:26:149:34 | [:alpha:] | RegExpNamedCharacterProperty |
227253
| test.cpp:149:35:149:43 | [:digit:] | RegExpNamedCharacterProperty |
@@ -377,9 +403,25 @@ regExpNormalCharValue
377403
| test.cpp:128:27:128:27 | F | F |
378404
| test.cpp:128:37:128:37 | a | a |
379405
| test.cpp:128:39:128:39 | f | f |
406+
| test.cpp:131:25:131:25 | : | : |
407+
| test.cpp:131:26:131:26 | d | d |
408+
| test.cpp:131:27:131:27 | i | i |
409+
| test.cpp:131:28:131:28 | g | g |
410+
| test.cpp:131:29:131:29 | i | i |
411+
| test.cpp:131:30:131:30 | t | t |
412+
| test.cpp:131:31:131:31 | : | : |
413+
| test.cpp:134:25:134:25 | : | : |
414+
| test.cpp:134:26:134:26 | a | a |
415+
| test.cpp:134:27:134:27 | l | l |
416+
| test.cpp:134:28:134:28 | p | p |
417+
| test.cpp:134:29:134:29 | h | h |
418+
| test.cpp:134:30:134:30 | a | a |
419+
| test.cpp:134:31:134:31 | : | : |
380420
| test.cpp:137:24:137:24 | a | a |
421+
| test.cpp:137:26:137:26 | : | : |
422+
| test.cpp:137:27:137:27 | b | b |
423+
| test.cpp:137:28:137:28 | : | : |
381424
| test.cpp:137:30:137:30 | c | c |
382-
| test.cpp:140:34:140:34 | - | - |
383425
| test.cpp:140:35:140:35 | z | z |
384426
| test.cpp:143:25:143:25 | [ | [ |
385427
| test.cpp:143:26:143:26 | : | : |
@@ -390,6 +432,15 @@ regExpNormalCharValue
390432
| test.cpp:143:31:143:31 | a | a |
391433
| test.cpp:146:25:146:25 | ] | ] |
392434
| test.cpp:146:26:146:26 | a | a |
435+
| test.cpp:146:27:146:27 | [ | [ |
436+
| test.cpp:146:28:146:28 | : | : |
437+
| test.cpp:146:29:146:29 | a | a |
438+
| test.cpp:146:30:146:30 | l | l |
439+
| test.cpp:146:31:146:31 | p | p |
440+
| test.cpp:146:32:146:32 | h | h |
441+
| test.cpp:146:33:146:33 | a | a |
442+
| test.cpp:146:34:146:34 | : | : |
443+
| test.cpp:146:36:146:36 | ] | ] |
393444
| test.cpp:163:21:163:26 | \\u9879 | \u9879 |
394445
| test.cpp:171:23:171:23 | a | a |
395446
| test.cpp:171:25:171:25 | b | b |

0 commit comments

Comments
 (0)