GNU nano - Bugs: bug #55169, with an invalid quoting regex,...
You are not allowed to post comments on this tracker with your current authentication level.
bug #55169: with an invalid quoting regex, paragraph movement functions crash
Submitter: | David Lawrence Ramsey <dolorous> | ||
Submitted: | Wed 05 Dec 2018 06:12:18 PM UTC | ||
Severity: | 3 - Normal | Status: | Fixed |
Assigned to: | dolorous | Open/Closed: | Closed |
Jump to the original submission
Tue 11 Dec 2018 06:54:30 PM UTC, comment #12: |
David Lawrence Ramsey <dolorous> |
Tue 11 Dec 2018 06:49:28 PM UTC, comment #11: Bug #55207 is the relevant report about the fixbounds() thing. (I had looked at this before, but it fell through the cracks: it didn't make it onto my todo list.) |
Benno Schulenberg <bens> |
Tue 11 Dec 2018 06:35:32 PM UTC, comment #10: Oh, come on, don't exaggerate. There is a difference between 1) the user making a mistake and nano helpfully alerting the user to it (instead of crashing), and 2) the user trying something that doesn't make sense in the first place and it not having any effect (or throwing an error) on one of the BSDs or some other platform.
|
Benno Schulenberg <bens> |
Tue 11 Dec 2018 12:50:13 PM UTC, comment #9: I don't recall ever using word boundaries in regex searches at all, but that doesn't mean they wouldn't ever be used. And I don't have access to anything running one of the BSDs, so it wouldn't come up in my case anyway. I was thinking more of if someone models new code based on that regcomp() call, they'll inadvertently be introducing a bug down the line.
|
David Lawrence Ramsey <dolorous> |
Tue 11 Dec 2018 10:04:40 AM UTC, comment #8: Thanks. First patch applied, commit 6e3b9ac0, followed by some reshuffling.
|
Benno Schulenberg <bens> |
Mon 10 Dec 2018 08:43:33 PM UTC, comment #7: Attached new version 3, against git c0abcc6.
|
David Lawrence Ramsey <dolorous> |
Sun 09 Dec 2018 04:42:42 PM UTC, comment #6: Hmm... This duplicates some ten lines of code, twice. This is not acceptable.
|
Benno Schulenberg <bens> |
Sat 08 Dec 2018 12:03:40 AM UTC, comment #5: There's one minor oversight in the last patch set: a small memory leak. The routine in main() that handles the -Q/--quotestr option should free quotestr before exiting if quotestr is set to an invalid regular expression. I'm not in front of my development machine, though, so I can't fix this yet. |
David Lawrence Ramsey <dolorous> |
Thu 06 Dec 2018 02:58:05 AM UTC, comment #4: After more testing, attaching patch set version 2, still against the same git version.
|
David Lawrence Ramsey <dolorous> |
Wed 05 Dec 2018 11:09:30 PM UTC, comment #3: Argh. Try this patch set instead.
|
David Lawrence Ramsey <dolorous> |
Wed 05 Dec 2018 10:24:47 PM UTC, comment #2: The attached patch set against git 55537dc should do it; it was tricky, so some testing is probably warranted.
|
David Lawrence Ramsey <dolorous> |
Wed 05 Dec 2018 08:02:16 PM UTC, comment #1: Wow! That's an old bug! Even 2.2.6 and 2.1.10 and 2.0.6 segfault with your recipe.
|
Benno Schulenberg <bens> |
Wed 05 Dec 2018 06:12:18 PM UTC, original submission:
This can be reproduced as follows (assuming that nano is built with regular expression support):
|
David Lawrence Ramsey <dolorous> |
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 9 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2019-03-25 | bens | Open/Closed | Open | Closed | |
2018-12-11 | bens | Status | In Progress | Fixed | |
Assigned to | None | dolorous | |||
2018-12-10 | dolorous | Attached File | - | Added regex-cleanup-3.zip, #45618 | |
2018-12-09 | bens | Status | None | In Progress | |
Summary | with invalid regex strings, paragraph movement functions crash nano | with an invalid quoting regex, paragraph movement functions crash | |||
2018-12-06 | dolorous | Attached File | - | Added regex-cleanup-2.zip, #45588 | |
2018-12-05 | dolorous | Attached File | - | Added regex-cleanup.zip, #45587 | |
2018-12-05 | dolorous | Attached File | - | Added regex-cleanup.zip, #45586 |
Powered by Savane 3.13-d3ae.
Corresponding source code
It was not intended as exaggeration.
Other than a check for invalid syntax, there should be no policing of what the user tries to put in solely based on what's commonly used. Not doing the word boundary character replacement in that case because it's not common to use word boundary functions in the quoting string is effectively doing that.
As for whether what the user is trying to do with the quoting string makes sense or not, if the user puts in something nonsensical, that should be on them, and nano should at least be resilient enough to try to handle it as-is if it will only take a minimum of code to work around it: in this particular case, it's a one-line fix. If it were much more than that, it would be a different matter. And if the issue would cause a security problem or a major issue, then it would also be a different matter. In short, if the user wants to shoot themselves in the foot, it shouldn't be our job to police it, barring major issues.
But enough philosophy. You're right in that having gnulib handle this instead is a better idea.