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

Why JSON3.read is used both for file path and json string content, and blocks file after read? #272

Open
sairus7 opened this issue Oct 6, 2023 · 5 comments

Comments

@sairus7
Copy link

sairus7 commented Oct 6, 2023

Because of this I get read error for any non-existent file, and maybe some incorrect behaviour for file names that match JSON

using JSON3
JSON3.read("123") # is it file or value? now its a value
open("123", "w") do io
    write(io, """
    {
        "some":"data"
    }
    """)
end
JSON3.read("123") # but now this is file
rm("123") # unlink("123"): permission denied (EACCES)
@0x0f0f0f
Copy link

0x0f0f0f commented Aug 1, 2024

@quinnj ping. This is actually a pretty bad security vulnerability. Imagine that we have a server and a client:

# Server

julia> using Oxygen, JSON3, HTTP

julia> # Define a POST route to read JSON data
       @post "/data" function(req::HTTP.Request)
           # Parse the request body as JSON
           json_data = JSON3.read(String(req.body))
           
           # Process the JSON data as needed
           println("Received JSON data:", json_data)
           
           # Return a simple response
           return Dict("status" => "success", "message" => json_data)
       end

julia> serve()

If we send

$ curl -X POST -H "Content-Type: application/json" -d '{"test": 123}' http://127.0.0.1:8080/data
{"status":"success","message":{"test":123}}

This is alright. If we have a file on the server called ~/sensitive.json which contains

{"super_secret_password": "abcdefg"}

We can send a request such that

$ curl -X POST -H "Content-Type: application/json" -d 'sensitive.json' http://127.0.0.1:8080/data
{"status":"success","message":{"super_secret_password":"abcdefg"}}

@0x0f0f0f
Copy link

0x0f0f0f commented Aug 1, 2024

See how this is done for TOML for example. It's either parsefile, tryparsefile or parse and tryparse. The decision of whether it's a file path or a TOML string is done by the caller!

@0x0f0f0f
Copy link

0x0f0f0f commented Aug 1, 2024

@0x0f0f0f
Copy link

0x0f0f0f commented Aug 1, 2024

Cross ref #95

The solutions could be many:

  • Make two different functions, read_file and read. One reads a file and the other reads a string with the contents of the files, as TOML and YAML already do. Disallow reading from a file in read.
  • Add a kwarg, readfile=false. Backwards compatible but not really the nicest.
  • Leave read functionality as-is and add read_file and read_string.

@0x0f0f0f
Copy link

0x0f0f0f commented Aug 1, 2024

I'm happy to file a PR once we agree on a solution

perrutquist added a commit to perrutquist/StructuredOutputs.jl that referenced this issue Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants