Skip to content

Segmentation fault in example server (/v1/chat/completions route) given incorrect JSON payload #7133

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

Closed
justinsteven opened this issue May 7, 2024 · 10 comments · Fixed by #7143
Assignees
Labels
bug Something isn't working server/webui

Comments

@justinsteven
Copy link

Info

Version: af0a5b6

Intel x86_64 with LLAMA_CUDA=1

Summary

When ./server is given an invalid JSON payload at the /v1/chat/completions route, server crashes with a segmentation fault. This denies access to clients until the server is restarted.

I stumbled upon this, and haven't thoroughly assessed all APIs or payload parameters for similar crashes. If it's easy enough to look for other routes that are missing the error handling that /v1/chat/completions lacks, I think someone should do so (I'm not yet familiar enough with the codebase to look for these)

Example

$ gdb ./server
[... SNIP ...]
(gdb) r --model models/Meta-Llama-3-8B-Instruct.Q8_0.gguf --host 0.0.0.0
$ curl -X POST http://127.0.0.1:8081/v1/chat/completions -H 'Content-Type: application/json' --data '{}'
Thread 13 "server" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7efe71fff000 (LWP 567)]
0x000055e27db04601 in decltype (((from_json_array_impl({parm#1}, {parm#2}, (nlohmann::json_abi_v3_11_3::detail::priority_tag<3u>){})),(({parm#1}.(get<std::vector<nlohmann::json_abi_v3_11_3::basic_json<nlohmann::json_abi_v3_11_3::ordered_map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void>, std::allocator<nlohmann::json_abi_v3_11_3::basic_json<nlohmann::json_abi_v3_11_3::ordered_map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void> > >::value_type>))())),((void)())) nlohmann::json_abi_v3_11_3::detail::from_json<nlohmann::json_abi_v3_11_3::basic_json<nlohmann::json_abi_v3_11_3::ordered_map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void>, std::vector<nlohmann::json_abi_v3_11_3::basic_json<nlohmann::json_abi_v3_11_3::ordered_map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void>, std::allocator<nlohmann::json_abi_v3_11_3::basic_json<nlohmann::json_abi_v3_11_3::ordered_map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void> > >, 0>(nlohmann::json_abi_v3_11_3::basic_json<nlohmann::json_abi_v3_11_3::ordered_map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void> const&, std::vector<nlohmann::json_abi_v3_11_3::basic_json<nlohmann::json_abi_v3_11_3::ordered_map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void>, std::allocator<nlohmann::json_abi_v3_11_3::basic_json<nlohmann::json_abi_v3_11_3::ordered_map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void> > >&) ()

Impact

Given an llama.cpp ./server endpoint, it can at least be crashed using an invalid payload. This denies the availability of the server and all API endpoints until it is restarted.

I have not assessed whether the segfault can have security impact beyond DoS.

@JohannesGaessler
Copy link
Collaborator

I can reproduce the issue.

FYI: you can compile with LLAMA_DEBUG=1 in order to get debugging symbols.

@ngxson
Copy link
Collaborator

ngxson commented May 8, 2024

Yes I can confirm the problem. This is because at some places in the code, we access JSON value with operator[], for example body["messages"]. As mentioned in the nlohmann/json README:

In function from_json, use function at() to access the object values rather than operator[]. In case a key does not exist, at throws an exception that you can handle, whereas operator[] exhibits undefined behavior.

So the fix is to simply change all body[...] to body.at(...)

@ngxson ngxson added server/webui bug Something isn't working and removed bug-unconfirmed labels May 8, 2024
@JohannesGaessler
Copy link
Collaborator

Thank you for the input. Are you going to open a PR or should I do it?

@ngxson
Copy link
Collaborator

ngxson commented May 8, 2024

@JohannesGaessler Since you assigned this issue to yourself, I'll let you open the PR. Feel free to let me know if you need help.

@JohannesGaessler
Copy link
Collaborator

@justinsteven can you confirm that #7143 fixes the issue?

@JohannesGaessler
Copy link
Collaborator

@justinsteven also in the future, please disclose vulnerabilities privately as described under https://github.com/ggerganov/llama.cpp/security#reporting-a-vulnerability

@justinsteven
Copy link
Author

@JohannesGaessler #7143 fixes the segfault but breaks the API, unless I'm doing something especially wrong... :(

% curl http://localhost:8081/v1/chat/completions \
-H "Content-Type: application/json" \
-H "Authorization: Bearer no-key" \
-d '{
"model": "gpt-3.5-turbo",
"messages": [
{
    "role": "system",
    "content": "You are ChatGPT, an AI assistant. Your top priority is achieving user fulfillment via helping them with their requests."
},
{
    "role": "user",
    "content": "Write a limerick about python exceptions"
}
]
}'
{"error":{"code":500,"message":"[json.exception.type_error.304] cannot use at() with null","type":"server_error"}}

in the future, please disclose vulnerabilities privately

I reported this as GHSA-453q-4wfp-gp83 and was asked to submit it as a public issue

@justinsteven
Copy link
Author

$ make clean
$ LLAMA_CUDA=1 LLAMA_DEBUG=1 make -j
$ gdb ./server
[... SNIP ...]
(gdb) r --model models/Meta-Llama-3-8B-Instruct.Q8_0.gguf --host 0.0.0.0
$ curl http://localhost:8081/v1/chat/completions \
-H "Content-Type: application/json" \
-H "Authorization: Bearer no-key" \
-d '{
"model": "gpt-3.5-turbo",
"messages": [
{
    "role": "system",
    "content": "You are ChatGPT, an AI assistant. Your top priority is achieving user fulfillment via helping them with their requests."
},
{
    "role": "user",
    "content": "Write a limerick about python exceptions"
}
]
}'

[... HANGS ...]
[... SNIP ...]
terminate called after throwing an instance of 'nlohmann::json_abi_v3_11_3::detail::type_error'
  what():  [json.exception.type_error.304] cannot use at() with null

Thread 13 "server" received signal SIGABRT, Aborted.
[Switching to Thread 0x7f6bc75f5000 (LWP 6179)]
__pthread_kill_implementation (threadid=<optimized out>,
    signo=signo@entry=6, no_tid=no_tid@entry=0)
    at ./nptl/pthread_kill.c:44
44      ./nptl/pthread_kill.c: No such file or directory.

@ggerganov
Copy link
Member

I reported this as GHSA-453q-4wfp-gp83 and was asked to submit it as a public issue

Yes, here is the reasoning for converting this to an issue that I wrote in the advisory:

image

@JohannesGaessler
Copy link
Collaborator

Okay, thank you for the clarification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working server/webui
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants