Skip to content
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.

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

Fix quoting bug for trailing \! for C shell #675

Merged
merged 4 commits into from
Jan 22, 2023
Merged

Conversation

ericcornelissen
Copy link
Owner

@ericcornelissen ericcornelissen commented Jan 22, 2023

Relates to #659
Found by nightly fuzzing (Jan 22)

Summary

Quoting for C shell does not work as expected when the argument ends with the substring "\!". After quoting such an argument, the resulting argument interpreted by C shell will be "!", while after quoting should still be "\!".

This bug has no security implications, it may just result in unexpected strings being passed to the program being invoked.

@ericcornelissen ericcornelissen added the bug Something isn't working label Jan 22, 2023
Update the test fixtures for `csh` to include examples of arguments with
the substring `"\!"` to ensure they are escaped properly. In particular,
the test case `"a\!"` is currently not escaped correctly by Shescape, so
this test acts as a regression test. The other tests are included for
completeness and to avoid regression against the current implementation
when fixing the current bug.

Additionally, include a new fuzz corpus entry which shows that it is
really only trailing `"\!"` substrings that aren't handled correctly. It
will blow up if `"\!"` in the middle of an argument is over-escaped.
@codecov
Copy link

codecov bot commented Jan 22, 2023

Codecov Report

Merging #675 (acc75b5) into main (5711982) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #675   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          901       904    +3     
=========================================
+ Hits           901       904    +3     
Flag Coverage Δ
integration-Ubuntu 90.37% <60.00%> (+0.03%) ⬆️
integration-Windows 83.84% <0.00%> (-0.28%) ⬇️
unit 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/unix.js 100.00% <100.00%> (ø)

@ericcornelissen ericcornelissen enabled auto-merge (squash) January 22, 2023 12:59
@ericcornelissen ericcornelissen changed the title Add CI fuzzing crash for csh Fix quoting bug for trailing \! for C shell Jan 22, 2023
@ericcornelissen ericcornelissen merged commit a3630f2 into main Jan 22, 2023
@ericcornelissen ericcornelissen deleted the csh-fuzz-crash branch January 22, 2023 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant