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.

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

Jump to bottom

stor insert does not escape single quotes #12764

Closed
lizclipse opened this issue May 4, 2024 · 2 comments · Fixed by #12820
Closed

stor insert does not escape single quotes #12764

lizclipse opened this issue May 4, 2024 · 2 comments · Fixed by #12820
Labels
good first issue Good for newcomers needs-triage An issue that hasn't had any proper look
Milestone
v0.94.0

Comments

@lizclipse
Copy link

Describe the bug

When using stor insert to add a row, it will not escape any strings that contain single quotes.

How to reproduce

$ stor create -t test -c { value: str }
$ stor insert -t history -d { command_line: "'hello world'" }
Error:   × Failed to open SQLite connection in memory from insert
   ╭─[source:1:1]
 1 │ nu --login
   · ▲
   · ╰── near "hello": syntax error in INSERT INTO history ( command_line) VALUES ( ''hello world'') at offset 47
   ╰────

Expected behavior

I'd expect strings to be properly escaped so that pipelines don't accidentally cause SQL injection bugs.

Screenshots

No response

Configuration

key value
version 0.93.0
major 0
minor 93
patch 0
branch
commit_hash
build_os macos-aarch64
build_target aarch64-apple-darwin
rust_version rustc 1.77.2 (25ef9e3d8 2024-04-09) (Homebrew)
cargo_version cargo 1.77.2
build_time 2024-04-30 22:51:13 +00:00
build_rust_channel release
allocator mimalloc
features dataframe, default, sqlite, system-clipboard, trash, which
installed_plugins ulid

Additional context

You can currently workaround this by feeding any input strings through

str replace "'" "''" -a
@lizclipse lizclipse added the needs-triage An issue that hasn't had any proper look label May 4, 2024
@fdncred fdncred added the good first issue Good for newcomers label May 4, 2024
@fdncred
Copy link
Collaborator

fdncred commented May 4, 2024

Not surprised. We'd accept a PR to fix this. Thanks.

@ExaltedBagel
Copy link
Contributor

I think I can take a quick look at this one

@ExaltedBagel ExaltedBagel mentioned this issue May 10, 2024
Merged
fdncred pushed a commit that referenced this issue May 14, 2024
@ExaltedBagel
Fix improperly escaped strings in stor insert (#12820)
cd381b7 
- fixes #12764 

Replaced the custom logic with values_to_sql method that is already used
in crate::database.
This will ensure that handling of parameters is the same between sqlite
and stor.
@hustcer hustcer added this to the v0.94.0 milestone May 14, 2024
FilipAndersson245 pushed a commit to FilipAndersson245/nushell that referenced this issue May 18, 2024
@ExaltedBagel @FilipAndersson245
Fix improperly escaped strings in stor insert (nushell#12820)
da51734 
- fixes nushell#12764 

Replaced the custom logic with values_to_sql method that is already used
in crate::database.
This will ensure that handling of parameters is the same between sqlite
and stor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers needs-triage An issue that hasn't had any proper look
Projects
None yet
Milestone
v0.94.0
Development

Successfully merging a pull request may close this issue.

Fix improperly escaped strings in stor insert ExaltedBagel/nushell
4 participants
@fdncred @hustcer @ExaltedBagel @lizclipse