Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries#74525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
olsaarik merged 6 commits into from
Sep 8, 2022
Merged

Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries #74525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fab6701
Test for exposing timeout check bug
olsaarik Aug 23, 2022
6f7c0b3
Fix timeout check bug
olsaarik Aug 24, 2022
56959d1
Improve naming
olsaarik Aug 24, 2022
db627c0
Simplify test
olsaarik Aug 24, 2022
8a3a702
Make test search range higher
olsaarik Aug 24, 2022
8fb04b9
Simplify test for timeout check bug
olsaarik Sep 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions ...xt.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexMatcher.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -448,13 +448,13 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
// still check the timeout now and again to provide some semblance of the behavior a developer experiences with
// the backtracking engines. We can, however, choose a large number here, since it's not actually needed for security.
const int CharsPerTimeoutCheck = 1_000;
ReadOnlySpan<char> inputForInnerLoop = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
input.Slice(0, pos + CharsPerTimeoutCheck) :
input;
int innerLoopLength = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
pos + CharsPerTimeoutCheck :
input.Length;

bool done = currentState.NfaState is not null ?
FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);
FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);

// If the inner loop indicates that the search finished (for example due to reaching a deadend state) or
// there is no more input available, then the whole search is done.
Expand All@@ -466,7 +466,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
// The search did not finish, so we either failed to transition (which should only happen if we were in DFA mode and
// need to switch over to NFA mode) or ran out of input in the inner loop. Check if the inner loop still had more
// input available.
if (pos < inputForInnerLoop.Length)
if (pos < innerLoopLength)
{
// Because there was still more input available, a failure to transition in DFA mode must be the cause
// of the early exit. Upgrade to NFA mode.
Expand DownExpand Up@@ -505,7 +505,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
/// 0 if iteration completed because we reached an initial state.
/// A negative value if iteration completed because we ran out of input or we failed to transition.
/// </returns>
private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, RegexRunnerMode mode,
private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, int length, RegexRunnerMode mode,
ref int posRef, ref CurrentState state, ref int endPosRef, ref int endStateIdRef, ref int initialStatePosRef, ref int initialStatePosCandidateRef)
where TStateHandler : struct, IStateHandler
where TInputReader : struct, IInputReader
Expand DownExpand Up@@ -561,7 +561,7 @@ private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizatio
}

// If there is more input available try to transition with the next character.
if (!IsMintermId(positionId) || !TStateHandler.TryTakeTransition(this, ref state, positionId))
if (pos >= length || !TStateHandler.TryTakeTransition(this, ref state, positionId))

@joperezrjoperezrAug 29, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understood correctly the bug was here where we were not checking if we are at the end of the input, right? I'm still not understanding why the problem is only happening for anchors

@olsaarikolsaarikSep 8, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was specifically with end anchors, as they rely on the position ID of the next character being -1 exactly when it is the end of the whole input. When they inadvertently matched, they would cause the whole search to end prematurely instead of the outer loop just checking the timeout and going back in for more input.

{
return false;
}
Expand Down
16 changes: 16 additions & 0 deletions src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1269,6 +1269,22 @@ public void Match_CachedPattern_NewTimeoutApplies(RegexOptions options)
Assert.InRange(sw.Elapsed.TotalSeconds, 0, 10); // arbitrary upper bound that should be well above what's needed with a 1ms timeout
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
public void NonBacktracking_NoEndAnchorMatchAtTimeoutCheck()
{
// This constant must be at least as large as the one in the implementation that sets the maximum number
// of innermost loop iterations between timeout checks.
const int CharsToTriggerTimeoutCheck = 10000;
// Check that it is indeed large enough to trigger timeouts. If this fails the constant above needs to be larger.
Assert.Throws<RegexMatchTimeoutException>(() => new Regex("a*", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromTicks(1))
.Match(new string('a', CharsToTriggerTimeoutCheck)));

// The actual test: ^a*$ shouldn't match in a string ending in 'b'
Regex testPattern = new Regex("^a*$", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromHours(1));
string input = string.Concat(new string('a', CharsToTriggerTimeoutCheck), 'b');
Assert.False(testPattern.IsMatch(input));
}

public static IEnumerable<object[]> Match_Advanced_TestData()
{
foreach (RegexEngine engine in RegexHelpers.AvailableEngines)
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries#74525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

olsaarik
merged 6 commits into

from

Sep 8, 2022
 Merged


Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries
#74525
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
fab6701

Test for exposing timeout check bug


olsaarik Aug 23, 2022
6f7c0b3

Fix timeout check bug


olsaarik Aug 24, 2022
56959d1

Improve naming


olsaarik Aug 24, 2022
db627c0

Simplify test


olsaarik Aug 24, 2022
8a3a702

Make test search range higher


olsaarik Aug 24, 2022
8fb04b9

Simplify test for timeout check bug


olsaarik Sep 8, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


16 changes: 8 additions & 8 deletions
...xt.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexMatcher.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -448,13 +448,13 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // still check the timeout now and again to provide some semblance of the behavior a developer experiences with
 // the backtracking engines. We can, however, choose a large number here, since it's not actually needed for security.
 const int CharsPerTimeoutCheck = 1_000;
ReadOnlySpan<char> inputForInnerLoop = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
input.Slice(0, pos + CharsPerTimeoutCheck) :
 input;
int innerLoopLength = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
 pos + CharsPerTimeoutCheck :
 input.Length;

 bool done = currentState.NfaState is not null ?
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);

 // If the inner loop indicates that the search finished (for example due to reaching a deadend state) or
 // there is no more input available, then the whole search is done.
Expand All@@ -466,7 +466,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // The search did not finish, so we either failed to transition (which should only happen if we were in DFA mode and
 // need to switch over to NFA mode) or ran out of input in the inner loop. Check if the inner loop still had more
 // input available.
 if (pos < inputForInnerLoop.Length)
 if (pos < innerLoopLength)
 {
 // Because there was still more input available, a failure to transition in DFA mode must be the cause
 // of the early exit. Upgrade to NFA mode.
Expand DownExpand Up@@ -505,7 +505,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 /// 0 if iteration completed because we reached an initial state.
 /// A negative value if iteration completed because we ran out of input or we failed to transition.
 /// </returns>
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, RegexRunnerMode mode,
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, int length, RegexRunnerMode mode,
 ref int posRef, ref CurrentState state, ref int endPosRef, ref int endStateIdRef, ref int initialStatePosRef, ref int initialStatePosCandidateRef)
 where TStateHandler : struct, IStateHandler
 where TInputReader : struct, IInputReader
Expand DownExpand Up@@ -561,7 +561,7 @@ private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizatio
 }

 // If there is more input available try to transition with the next character.
 if (!IsMintermId(positionId) || !TStateHandler.TryTakeTransition(this, ref state, positionId))
 if (pos >= length || !TStateHandler.TryTakeTransition(this, ref state, positionId))
@joperezrjoperezrAug 29, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understood correctly the bug was here where we were not checking if we are at the end of the input, right? I'm still not understanding why the problem is only happening for anchors
@olsaarikolsaarikSep 8, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was specifically with end anchors, as they rely on the position ID of the next character being -1 exactly when it is the end of the whole input. When they inadvertently matched, they would cause the whole search to end prematurely instead of the outer loop just checking the timeout and going back in for more input.
 {
 return false;
 }
Expand Down

16 changes: 16 additions & 0 deletions
src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1269,6 +1269,22 @@ public void Match_CachedPattern_NewTimeoutApplies(RegexOptions options)
 Assert.InRange(sw.Elapsed.TotalSeconds, 0, 10); // arbitrary upper bound that should be well above what's needed with a 1ms timeout
 }

 [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
 public void NonBacktracking_NoEndAnchorMatchAtTimeoutCheck()
 {
 // This constant must be at least as large as the one in the implementation that sets the maximum number
 // of innermost loop iterations between timeout checks.
 const int CharsToTriggerTimeoutCheck = 10000;
 // Check that it is indeed large enough to trigger timeouts. If this fails the constant above needs to be larger.
 Assert.Throws<RegexMatchTimeoutException>(() => new Regex("a*", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromTicks(1))
 .Match(new string('a', CharsToTriggerTimeoutCheck)));

 // The actual test: ^a*$ shouldn't match in a string ending in 'b'
 Regex testPattern = new Regex("^a*$", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromHours(1));
 string input = string.Concat(new string('a', CharsToTriggerTimeoutCheck), 'b');
 Assert.False(testPattern.IsMatch(input));
 }

 public static IEnumerable<object[]> Match_Advanced_TestData()
 {
 foreach (RegexEngine engine in RegexHelpers.AvailableEngines)
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries#74525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

olsaarik
merged 6 commits into

from

Sep 8, 2022
 Merged


Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries
#74525
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
fab6701

Test for exposing timeout check bug


olsaarik Aug 23, 2022
6f7c0b3

Fix timeout check bug


olsaarik Aug 24, 2022
56959d1

Improve naming


olsaarik Aug 24, 2022
db627c0

Simplify test


olsaarik Aug 24, 2022
8a3a702

Make test search range higher


olsaarik Aug 24, 2022
8fb04b9

Simplify test for timeout check bug


olsaarik Sep 8, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


16 changes: 8 additions & 8 deletions
...xt.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexMatcher.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -448,13 +448,13 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // still check the timeout now and again to provide some semblance of the behavior a developer experiences with
 // the backtracking engines. We can, however, choose a large number here, since it's not actually needed for security.
 const int CharsPerTimeoutCheck = 1_000;
ReadOnlySpan<char> inputForInnerLoop = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
input.Slice(0, pos + CharsPerTimeoutCheck) :
 input;
int innerLoopLength = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
 pos + CharsPerTimeoutCheck :
 input.Length;

 bool done = currentState.NfaState is not null ?
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);

 // If the inner loop indicates that the search finished (for example due to reaching a deadend state) or
 // there is no more input available, then the whole search is done.
Expand All@@ -466,7 +466,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // The search did not finish, so we either failed to transition (which should only happen if we were in DFA mode and
 // need to switch over to NFA mode) or ran out of input in the inner loop. Check if the inner loop still had more
 // input available.
 if (pos < inputForInnerLoop.Length)
 if (pos < innerLoopLength)
 {
 // Because there was still more input available, a failure to transition in DFA mode must be the cause
 // of the early exit. Upgrade to NFA mode.
Expand DownExpand Up@@ -505,7 +505,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 /// 0 if iteration completed because we reached an initial state.
 /// A negative value if iteration completed because we ran out of input or we failed to transition.
 /// </returns>
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, RegexRunnerMode mode,
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, int length, RegexRunnerMode mode,
 ref int posRef, ref CurrentState state, ref int endPosRef, ref int endStateIdRef, ref int initialStatePosRef, ref int initialStatePosCandidateRef)
 where TStateHandler : struct, IStateHandler
 where TInputReader : struct, IInputReader
Expand DownExpand Up@@ -561,7 +561,7 @@ private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizatio
 }

 // If there is more input available try to transition with the next character.
 if (!IsMintermId(positionId) || !TStateHandler.TryTakeTransition(this, ref state, positionId))
 if (pos >= length || !TStateHandler.TryTakeTransition(this, ref state, positionId))
@joperezrjoperezrAug 29, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understood correctly the bug was here where we were not checking if we are at the end of the input, right? I'm still not understanding why the problem is only happening for anchors
@olsaarikolsaarikSep 8, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was specifically with end anchors, as they rely on the position ID of the next character being -1 exactly when it is the end of the whole input. When they inadvertently matched, they would cause the whole search to end prematurely instead of the outer loop just checking the timeout and going back in for more input.
 {
 return false;
 }
Expand Down

16 changes: 16 additions & 0 deletions
src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1269,6 +1269,22 @@ public void Match_CachedPattern_NewTimeoutApplies(RegexOptions options)
 Assert.InRange(sw.Elapsed.TotalSeconds, 0, 10); // arbitrary upper bound that should be well above what's needed with a 1ms timeout
 }

 [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
 public void NonBacktracking_NoEndAnchorMatchAtTimeoutCheck()
 {
 // This constant must be at least as large as the one in the implementation that sets the maximum number
 // of innermost loop iterations between timeout checks.
 const int CharsToTriggerTimeoutCheck = 10000;
 // Check that it is indeed large enough to trigger timeouts. If this fails the constant above needs to be larger.
 Assert.Throws<RegexMatchTimeoutException>(() => new Regex("a*", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromTicks(1))
 .Match(new string('a', CharsToTriggerTimeoutCheck)));

 // The actual test: ^a*$ shouldn't match in a string ending in 'b'
 Regex testPattern = new Regex("^a*$", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromHours(1));
 string input = string.Concat(new string('a', CharsToTriggerTimeoutCheck), 'b');
 Assert.False(testPattern.IsMatch(input));
 }

 public static IEnumerable<object[]> Match_Advanced_TestData()
 {
 foreach (RegexEngine engine in RegexHelpers.AvailableEngines)
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries#74525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

olsaarik
merged 6 commits into

from

Sep 8, 2022
 Merged


Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries
#74525
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
fab6701

Test for exposing timeout check bug


olsaarik Aug 23, 2022
6f7c0b3

Fix timeout check bug


olsaarik Aug 24, 2022
56959d1

Improve naming


olsaarik Aug 24, 2022
db627c0

Simplify test


olsaarik Aug 24, 2022
8a3a702

Make test search range higher


olsaarik Aug 24, 2022
8fb04b9

Simplify test for timeout check bug


olsaarik Sep 8, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


16 changes: 8 additions & 8 deletions
...xt.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexMatcher.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -448,13 +448,13 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // still check the timeout now and again to provide some semblance of the behavior a developer experiences with
 // the backtracking engines. We can, however, choose a large number here, since it's not actually needed for security.
 const int CharsPerTimeoutCheck = 1_000;
ReadOnlySpan<char> inputForInnerLoop = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
input.Slice(0, pos + CharsPerTimeoutCheck) :
 input;
int innerLoopLength = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
 pos + CharsPerTimeoutCheck :
 input.Length;

 bool done = currentState.NfaState is not null ?
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);

 // If the inner loop indicates that the search finished (for example due to reaching a deadend state) or
 // there is no more input available, then the whole search is done.
Expand All@@ -466,7 +466,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // The search did not finish, so we either failed to transition (which should only happen if we were in DFA mode and
 // need to switch over to NFA mode) or ran out of input in the inner loop. Check if the inner loop still had more
 // input available.
 if (pos < inputForInnerLoop.Length)
 if (pos < innerLoopLength)
 {
 // Because there was still more input available, a failure to transition in DFA mode must be the cause
 // of the early exit. Upgrade to NFA mode.
Expand DownExpand Up@@ -505,7 +505,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 /// 0 if iteration completed because we reached an initial state.
 /// A negative value if iteration completed because we ran out of input or we failed to transition.
 /// </returns>
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, RegexRunnerMode mode,
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, int length, RegexRunnerMode mode,
 ref int posRef, ref CurrentState state, ref int endPosRef, ref int endStateIdRef, ref int initialStatePosRef, ref int initialStatePosCandidateRef)
 where TStateHandler : struct, IStateHandler
 where TInputReader : struct, IInputReader
Expand DownExpand Up@@ -561,7 +561,7 @@ private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizatio
 }

 // If there is more input available try to transition with the next character.
 if (!IsMintermId(positionId) || !TStateHandler.TryTakeTransition(this, ref state, positionId))
 if (pos >= length || !TStateHandler.TryTakeTransition(this, ref state, positionId))
@joperezrjoperezrAug 29, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understood correctly the bug was here where we were not checking if we are at the end of the input, right? I'm still not understanding why the problem is only happening for anchors
@olsaarikolsaarikSep 8, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was specifically with end anchors, as they rely on the position ID of the next character being -1 exactly when it is the end of the whole input. When they inadvertently matched, they would cause the whole search to end prematurely instead of the outer loop just checking the timeout and going back in for more input.
 {
 return false;
 }
Expand Down

16 changes: 16 additions & 0 deletions
src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1269,6 +1269,22 @@ public void Match_CachedPattern_NewTimeoutApplies(RegexOptions options)
 Assert.InRange(sw.Elapsed.TotalSeconds, 0, 10); // arbitrary upper bound that should be well above what's needed with a 1ms timeout
 }

 [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
 public void NonBacktracking_NoEndAnchorMatchAtTimeoutCheck()
 {
 // This constant must be at least as large as the one in the implementation that sets the maximum number
 // of innermost loop iterations between timeout checks.
 const int CharsToTriggerTimeoutCheck = 10000;
 // Check that it is indeed large enough to trigger timeouts. If this fails the constant above needs to be larger.
 Assert.Throws<RegexMatchTimeoutException>(() => new Regex("a*", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromTicks(1))
 .Match(new string('a', CharsToTriggerTimeoutCheck)));

 // The actual test: ^a*$ shouldn't match in a string ending in 'b'
 Regex testPattern = new Regex("^a*$", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromHours(1));
 string input = string.Concat(new string('a', CharsToTriggerTimeoutCheck), 'b');
 Assert.False(testPattern.IsMatch(input));
 }

 public static IEnumerable<object[]> Match_Advanced_TestData()
 {
 foreach (RegexEngine engine in RegexHelpers.AvailableEngines)
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries#74525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

olsaarik
merged 6 commits into

from

Sep 8, 2022
 Merged


Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries
#74525
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
fab6701

Test for exposing timeout check bug


olsaarik Aug 23, 2022
6f7c0b3

Fix timeout check bug


olsaarik Aug 24, 2022
56959d1

Improve naming


olsaarik Aug 24, 2022
db627c0

Simplify test


olsaarik Aug 24, 2022
8a3a702

Make test search range higher


olsaarik Aug 24, 2022
8fb04b9

Simplify test for timeout check bug


olsaarik Sep 8, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


16 changes: 8 additions & 8 deletions
...xt.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexMatcher.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -448,13 +448,13 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // still check the timeout now and again to provide some semblance of the behavior a developer experiences with
 // the backtracking engines. We can, however, choose a large number here, since it's not actually needed for security.
 const int CharsPerTimeoutCheck = 1_000;
ReadOnlySpan<char> inputForInnerLoop = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
input.Slice(0, pos + CharsPerTimeoutCheck) :
 input;
int innerLoopLength = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
 pos + CharsPerTimeoutCheck :
 input.Length;

 bool done = currentState.NfaState is not null ?
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);

 // If the inner loop indicates that the search finished (for example due to reaching a deadend state) or
 // there is no more input available, then the whole search is done.
Expand All@@ -466,7 +466,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // The search did not finish, so we either failed to transition (which should only happen if we were in DFA mode and
 // need to switch over to NFA mode) or ran out of input in the inner loop. Check if the inner loop still had more
 // input available.
 if (pos < inputForInnerLoop.Length)
 if (pos < innerLoopLength)
 {
 // Because there was still more input available, a failure to transition in DFA mode must be the cause
 // of the early exit. Upgrade to NFA mode.
Expand DownExpand Up@@ -505,7 +505,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 /// 0 if iteration completed because we reached an initial state.
 /// A negative value if iteration completed because we ran out of input or we failed to transition.
 /// </returns>
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, RegexRunnerMode mode,
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, int length, RegexRunnerMode mode,
 ref int posRef, ref CurrentState state, ref int endPosRef, ref int endStateIdRef, ref int initialStatePosRef, ref int initialStatePosCandidateRef)
 where TStateHandler : struct, IStateHandler
 where TInputReader : struct, IInputReader
Expand DownExpand Up@@ -561,7 +561,7 @@ private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizatio
 }

 // If there is more input available try to transition with the next character.
 if (!IsMintermId(positionId) || !TStateHandler.TryTakeTransition(this, ref state, positionId))
 if (pos >= length || !TStateHandler.TryTakeTransition(this, ref state, positionId))
@joperezrjoperezrAug 29, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understood correctly the bug was here where we were not checking if we are at the end of the input, right? I'm still not understanding why the problem is only happening for anchors
@olsaarikolsaarikSep 8, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was specifically with end anchors, as they rely on the position ID of the next character being -1 exactly when it is the end of the whole input. When they inadvertently matched, they would cause the whole search to end prematurely instead of the outer loop just checking the timeout and going back in for more input.
 {
 return false;
 }
Expand Down

16 changes: 16 additions & 0 deletions
src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1269,6 +1269,22 @@ public void Match_CachedPattern_NewTimeoutApplies(RegexOptions options)
 Assert.InRange(sw.Elapsed.TotalSeconds, 0, 10); // arbitrary upper bound that should be well above what's needed with a 1ms timeout
 }

 [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
 public void NonBacktracking_NoEndAnchorMatchAtTimeoutCheck()
 {
 // This constant must be at least as large as the one in the implementation that sets the maximum number
 // of innermost loop iterations between timeout checks.
 const int CharsToTriggerTimeoutCheck = 10000;
 // Check that it is indeed large enough to trigger timeouts. If this fails the constant above needs to be larger.
 Assert.Throws<RegexMatchTimeoutException>(() => new Regex("a*", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromTicks(1))
 .Match(new string('a', CharsToTriggerTimeoutCheck)));

 // The actual test: ^a*$ shouldn't match in a string ending in 'b'
 Regex testPattern = new Regex("^a*$", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromHours(1));
 string input = string.Concat(new string('a', CharsToTriggerTimeoutCheck), 'b');
 Assert.False(testPattern.IsMatch(input));
 }

 public static IEnumerable<object[]> Match_Advanced_TestData()
 {
 foreach (RegexEngine engine in RegexHelpers.AvailableEngines)
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries#74525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

olsaarik
merged 6 commits into

from

Sep 8, 2022
 Merged


Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries
#74525
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
fab6701

Test for exposing timeout check bug


olsaarik Aug 23, 2022
6f7c0b3

Fix timeout check bug


olsaarik Aug 24, 2022
56959d1

Improve naming


olsaarik Aug 24, 2022
db627c0

Simplify test


olsaarik Aug 24, 2022
8a3a702

Make test search range higher


olsaarik Aug 24, 2022
8fb04b9

Simplify test for timeout check bug


olsaarik Sep 8, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


16 changes: 8 additions & 8 deletions
...xt.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexMatcher.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -448,13 +448,13 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // still check the timeout now and again to provide some semblance of the behavior a developer experiences with
 // the backtracking engines. We can, however, choose a large number here, since it's not actually needed for security.
 const int CharsPerTimeoutCheck = 1_000;
ReadOnlySpan<char> inputForInnerLoop = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
input.Slice(0, pos + CharsPerTimeoutCheck) :
 input;
int innerLoopLength = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
 pos + CharsPerTimeoutCheck :
 input.Length;

 bool done = currentState.NfaState is not null ?
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);

 // If the inner loop indicates that the search finished (for example due to reaching a deadend state) or
 // there is no more input available, then the whole search is done.
Expand All@@ -466,7 +466,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // The search did not finish, so we either failed to transition (which should only happen if we were in DFA mode and
 // need to switch over to NFA mode) or ran out of input in the inner loop. Check if the inner loop still had more
 // input available.
 if (pos < inputForInnerLoop.Length)
 if (pos < innerLoopLength)
 {
 // Because there was still more input available, a failure to transition in DFA mode must be the cause
 // of the early exit. Upgrade to NFA mode.
Expand DownExpand Up@@ -505,7 +505,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 /// 0 if iteration completed because we reached an initial state.
 /// A negative value if iteration completed because we ran out of input or we failed to transition.
 /// </returns>
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, RegexRunnerMode mode,
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, int length, RegexRunnerMode mode,
 ref int posRef, ref CurrentState state, ref int endPosRef, ref int endStateIdRef, ref int initialStatePosRef, ref int initialStatePosCandidateRef)
 where TStateHandler : struct, IStateHandler
 where TInputReader : struct, IInputReader
Expand DownExpand Up@@ -561,7 +561,7 @@ private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizatio
 }

 // If there is more input available try to transition with the next character.
 if (!IsMintermId(positionId) || !TStateHandler.TryTakeTransition(this, ref state, positionId))
 if (pos >= length || !TStateHandler.TryTakeTransition(this, ref state, positionId))
@joperezrjoperezrAug 29, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understood correctly the bug was here where we were not checking if we are at the end of the input, right? I'm still not understanding why the problem is only happening for anchors
@olsaarikolsaarikSep 8, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was specifically with end anchors, as they rely on the position ID of the next character being -1 exactly when it is the end of the whole input. When they inadvertently matched, they would cause the whole search to end prematurely instead of the outer loop just checking the timeout and going back in for more input.
 {
 return false;
 }
Expand Down

16 changes: 16 additions & 0 deletions
src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1269,6 +1269,22 @@ public void Match_CachedPattern_NewTimeoutApplies(RegexOptions options)
 Assert.InRange(sw.Elapsed.TotalSeconds, 0, 10); // arbitrary upper bound that should be well above what's needed with a 1ms timeout
 }

 [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
 public void NonBacktracking_NoEndAnchorMatchAtTimeoutCheck()
 {
 // This constant must be at least as large as the one in the implementation that sets the maximum number
 // of innermost loop iterations between timeout checks.
 const int CharsToTriggerTimeoutCheck = 10000;
 // Check that it is indeed large enough to trigger timeouts. If this fails the constant above needs to be larger.
 Assert.Throws<RegexMatchTimeoutException>(() => new Regex("a*", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromTicks(1))
 .Match(new string('a', CharsToTriggerTimeoutCheck)));

 // The actual test: ^a*$ shouldn't match in a string ending in 'b'
 Regex testPattern = new Regex("^a*$", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromHours(1));
 string input = string.Concat(new string('a', CharsToTriggerTimeoutCheck), 'b');
 Assert.False(testPattern.IsMatch(input));
 }

 public static IEnumerable<object[]> Match_Advanced_TestData()
 {
 foreach (RegexEngine engine in RegexHelpers.AvailableEngines)
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries#74525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

olsaarik
merged 6 commits into

from

Sep 8, 2022
 Merged


Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries
#74525
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
fab6701

Test for exposing timeout check bug


olsaarik Aug 23, 2022
6f7c0b3

Fix timeout check bug


olsaarik Aug 24, 2022
56959d1

Improve naming


olsaarik Aug 24, 2022
db627c0

Simplify test


olsaarik Aug 24, 2022
8a3a702

Make test search range higher


olsaarik Aug 24, 2022
8fb04b9

Simplify test for timeout check bug


olsaarik Sep 8, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


16 changes: 8 additions & 8 deletions
...xt.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexMatcher.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -448,13 +448,13 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // still check the timeout now and again to provide some semblance of the behavior a developer experiences with
 // the backtracking engines. We can, however, choose a large number here, since it's not actually needed for security.
 const int CharsPerTimeoutCheck = 1_000;
ReadOnlySpan<char> inputForInnerLoop = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
input.Slice(0, pos + CharsPerTimeoutCheck) :
 input;
int innerLoopLength = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
 pos + CharsPerTimeoutCheck :
 input.Length;

 bool done = currentState.NfaState is not null ?
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);

 // If the inner loop indicates that the search finished (for example due to reaching a deadend state) or
 // there is no more input available, then the whole search is done.
Expand All@@ -466,7 +466,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // The search did not finish, so we either failed to transition (which should only happen if we were in DFA mode and
 // need to switch over to NFA mode) or ran out of input in the inner loop. Check if the inner loop still had more
 // input available.
 if (pos < inputForInnerLoop.Length)
 if (pos < innerLoopLength)
 {
 // Because there was still more input available, a failure to transition in DFA mode must be the cause
 // of the early exit. Upgrade to NFA mode.
Expand DownExpand Up@@ -505,7 +505,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 /// 0 if iteration completed because we reached an initial state.
 /// A negative value if iteration completed because we ran out of input or we failed to transition.
 /// </returns>
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, RegexRunnerMode mode,
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, int length, RegexRunnerMode mode,
 ref int posRef, ref CurrentState state, ref int endPosRef, ref int endStateIdRef, ref int initialStatePosRef, ref int initialStatePosCandidateRef)
 where TStateHandler : struct, IStateHandler
 where TInputReader : struct, IInputReader
Expand DownExpand Up@@ -561,7 +561,7 @@ private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizatio
 }

 // If there is more input available try to transition with the next character.
 if (!IsMintermId(positionId) || !TStateHandler.TryTakeTransition(this, ref state, positionId))
 if (pos >= length || !TStateHandler.TryTakeTransition(this, ref state, positionId))
@joperezrjoperezrAug 29, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understood correctly the bug was here where we were not checking if we are at the end of the input, right? I'm still not understanding why the problem is only happening for anchors
@olsaarikolsaarikSep 8, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was specifically with end anchors, as they rely on the position ID of the next character being -1 exactly when it is the end of the whole input. When they inadvertently matched, they would cause the whole search to end prematurely instead of the outer loop just checking the timeout and going back in for more input.
 {
 return false;
 }
Expand Down

16 changes: 16 additions & 0 deletions
src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1269,6 +1269,22 @@ public void Match_CachedPattern_NewTimeoutApplies(RegexOptions options)
 Assert.InRange(sw.Elapsed.TotalSeconds, 0, 10); // arbitrary upper bound that should be well above what's needed with a 1ms timeout
 }

 [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
 public void NonBacktracking_NoEndAnchorMatchAtTimeoutCheck()
 {
 // This constant must be at least as large as the one in the implementation that sets the maximum number
 // of innermost loop iterations between timeout checks.
 const int CharsToTriggerTimeoutCheck = 10000;
 // Check that it is indeed large enough to trigger timeouts. If this fails the constant above needs to be larger.
 Assert.Throws<RegexMatchTimeoutException>(() => new Regex("a*", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromTicks(1))
 .Match(new string('a', CharsToTriggerTimeoutCheck)));

 // The actual test: ^a*$ shouldn't match in a string ending in 'b'
 Regex testPattern = new Regex("^a*$", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromHours(1));
 string input = string.Concat(new string('a', CharsToTriggerTimeoutCheck), 'b');
 Assert.False(testPattern.IsMatch(input));
 }

 public static IEnumerable<object[]> Match_Advanced_TestData()
 {
 foreach (RegexEngine engine in RegexHelpers.AvailableEngines)
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries#74525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

olsaarik
merged 6 commits into

from

Sep 8, 2022
 Merged


Fix RegexOptions.NonBacktracking matching end anchors at timeout check boundaries
#74525
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
fab6701

Test for exposing timeout check bug


olsaarik Aug 23, 2022
6f7c0b3

Fix timeout check bug


olsaarik Aug 24, 2022
56959d1

Improve naming


olsaarik Aug 24, 2022
db627c0

Simplify test


olsaarik Aug 24, 2022
8a3a702

Make test search range higher


olsaarik Aug 24, 2022
8fb04b9

Simplify test for timeout check bug


olsaarik Sep 8, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


16 changes: 8 additions & 8 deletions
...xt.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexMatcher.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -448,13 +448,13 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // still check the timeout now and again to provide some semblance of the behavior a developer experiences with
 // the backtracking engines. We can, however, choose a large number here, since it's not actually needed for security.
 const int CharsPerTimeoutCheck = 1_000;
ReadOnlySpan<char> inputForInnerLoop = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
input.Slice(0, pos + CharsPerTimeoutCheck) :
 input;
int innerLoopLength = _checkTimeout && input.Length - pos > CharsPerTimeoutCheck ?
 pos + CharsPerTimeoutCheck :
 input.Length;

 bool done = currentState.NfaState is not null ?
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(inputForInnerLoop, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);
 FindEndPositionDeltas<NfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate) :
 FindEndPositionDeltas<DfaStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(input, innerLoopLength, mode, ref pos, ref currentState, ref endPos, ref endStateId, ref initialStatePos, ref initialStatePosCandidate);

 // If the inner loop indicates that the search finished (for example due to reaching a deadend state) or
 // there is no more input available, then the whole search is done.
Expand All@@ -466,7 +466,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 // The search did not finish, so we either failed to transition (which should only happen if we were in DFA mode and
 // need to switch over to NFA mode) or ran out of input in the inner loop. Check if the inner loop still had more
 // input available.
 if (pos < inputForInnerLoop.Length)
 if (pos < innerLoopLength)
 {
 // Because there was still more input available, a failure to transition in DFA mode must be the cause
 // of the early exit. Upgrade to NFA mode.
Expand DownExpand Up@@ -505,7 +505,7 @@ private int FindEndPosition<TInputReader, TFindOptimizationsHandler, TNullabilit
 /// 0 if iteration completed because we reached an initial state.
 /// A negative value if iteration completed because we ran out of input or we failed to transition.
 /// </returns>
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, RegexRunnerMode mode,
 private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizationsHandler, TNullabilityHandler>(ReadOnlySpan<char> input, int length, RegexRunnerMode mode,
 ref int posRef, ref CurrentState state, ref int endPosRef, ref int endStateIdRef, ref int initialStatePosRef, ref int initialStatePosCandidateRef)
 where TStateHandler : struct, IStateHandler
 where TInputReader : struct, IInputReader
Expand DownExpand Up@@ -561,7 +561,7 @@ private bool FindEndPositionDeltas<TStateHandler, TInputReader, TFindOptimizatio
 }

 // If there is more input available try to transition with the next character.
 if (!IsMintermId(positionId) || !TStateHandler.TryTakeTransition(this, ref state, positionId))
 if (pos >= length || !TStateHandler.TryTakeTransition(this, ref state, positionId))
@joperezrjoperezrAug 29, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understood correctly the bug was here where we were not checking if we are at the end of the input, right? I'm still not understanding why the problem is only happening for anchors
@olsaarikolsaarikSep 8, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was specifically with end anchors, as they rely on the position ID of the next character being -1 exactly when it is the end of the whole input. When they inadvertently matched, they would cause the whole search to end prematurely instead of the outer loop just checking the timeout and going back in for more input.
 {
 return false;
 }
Expand Down

16 changes: 16 additions & 0 deletions
src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1269,6 +1269,22 @@ public void Match_CachedPattern_NewTimeoutApplies(RegexOptions options)
 Assert.InRange(sw.Elapsed.TotalSeconds, 0, 10); // arbitrary upper bound that should be well above what's needed with a 1ms timeout
 }

 [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
 public void NonBacktracking_NoEndAnchorMatchAtTimeoutCheck()
 {
 // This constant must be at least as large as the one in the implementation that sets the maximum number
 // of innermost loop iterations between timeout checks.
 const int CharsToTriggerTimeoutCheck = 10000;
 // Check that it is indeed large enough to trigger timeouts. If this fails the constant above needs to be larger.
 Assert.Throws<RegexMatchTimeoutException>(() => new Regex("a*", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromTicks(1))
 .Match(new string('a', CharsToTriggerTimeoutCheck)));

 // The actual test: ^a*$ shouldn't match in a string ending in 'b'
 Regex testPattern = new Regex("^a*$", RegexHelpers.RegexOptionNonBacktracking, TimeSpan.FromHours(1));
 string input = string.Concat(new string('a', CharsToTriggerTimeoutCheck), 'b');
 Assert.False(testPattern.IsMatch(input));
 }

 public static IEnumerable<object[]> Match_Advanced_TestData()
 {
 foreach (RegexEngine engine in RegexHelpers.AvailableEngines)
Expand Down