Buffer overflow when using reverse proxy via HttpService::Proxy

[fnMrRice]

• Version: vcpkg-v1.3.2
• Appearance: Sometimes reverse proxy may send invalid content to back server. With AddressSanitizer it would abort the program.

AddressSanitizer log:

==262128==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50c000030777 at pc 0x7ffff796e003 bp 0x7fffe6e88c90 sp 0x7fffe6e88438
READ of size 524 at 0x50c000030777 thread T12
    #0 0x7ffff796e002 in __interceptor_memcpy /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:899
    #1 0x7ffff77eb786 in std::char_traits<char>::copy(char*, char const*, unsigned long) /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:445
    #2 0x7ffff77eb786 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy(char*, char const*, unsigned long) /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:420
    #3 0x7ffff77eb786 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy(char*, char const*, unsigned long) /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:415
    #4 0x7ffff77eb786 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:333
    #5 0x7ffff77ed100 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_append(char const*, unsigned long) /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:419
    #6 0x5555557436dc in HttpMessage::DumpBody(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/HttpMessage.cpp:565
    #7 0x555555744fde in HttpRequest::Dump[abi:cxx11](bool, bool) /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/HttpMessage.cpp:789
    #8 0x5555557881bc in HttpHandler::sendProxyRequest() /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/server/HttpHandler.cpp:1085
    #9 0x555555787b23 in HttpHandler::connectProxy(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/server/HttpHandler.cpp:1020
    #10 0x5555557879bb in HttpHandler::handleReverseProxy() /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/server/HttpHandler.cpp:1007
    #11 0x555555787887 in HttpHandler::handleProxy() /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/server/HttpHandler.cpp:990
    #12 0x555555783300 in HttpHandler::onHeadersComplete() /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/server/HttpHandler.cpp:265
    #13 0x555555782341 in operator() /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/server/HttpHandler.cpp:95
    #14 0x555555789579 in __invoke_impl<void, HttpHandler::Init(int)::<lambda(HttpMessage*, http_parser_state, char const*, size_t)>&, HttpMessage*, http_parser_state, char const*, long unsigned int> /usr/include/c++/13.2.1/bits/invoke.h:61
    #15 0x555555789056 in __invoke_r<void, HttpHandler::Init(int)::<lambda(HttpMessage*, http_parser_state, char const*, size_t)>&, HttpMessage*, http_parser_state, char const*, long unsigned int> /usr/include/c++/13.2.1/bits/invoke.h:150
    #16 0x555555788b7b in _M_invoke /usr/include/c++/13.2.1/bits/std_function.h:290
    #17 0x5555557a22da in std::function<void (HttpMessage*, http_parser_state, char const*, unsigned long)>::operator()(HttpMessage*, http_parser_state, char const*, unsigned long) const /usr/include/c++/13.2.1/bits/std_function.h:591
    #18 0x5555557a1fcd in Http1Parser::invokeHttpCb(char const*, unsigned long) /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/Http1Parser.h:130
    #19 0x5555557a17cf in on_headers_complete /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/Http1Parser.cpp:128
    #20 0x5555557a6d5c in http_parser_execute /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/http_parser.c:1800
    #21 0x5555557a1d04 in Http1Parser::FeedRecvData(char const*, unsigned long) /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/Http1Parser.h:53
    #22 0x555555785b8e in HttpHandler::FeedRecvData(char const*, unsigned long) /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/server/HttpHandler.cpp:700
    #23 0x55555576ff74 in on_recv /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/server/HttpServer.cpp:30
    #24 0x555555736ed5 in hio_read_cb /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/event/hevent.c:404
    #25 0x555555736d78 in hio_handle_read /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/event/hevent.c:371
    #26 0x55555573ca53 in __read_cb /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/event/nio.c:48
    #27 0x55555573d688 in nio_read /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/event/nio.c:332
    #28 0x55555573d940 in hio_handle_events /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/event/nio.c:410
    #29 0x555555739c09 in hloop_process_pendings /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/event/hloop.c:122
    #30 0x555555739e9f in hloop_process_events /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/event/hloop.c:185
    #31 0x55555573a9c3 in hloop_run /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/event/hloop.c:464
    #32 0x55555568fa72 in hv::EventLoop::run() /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/evpp/EventLoop.h:54
    #33 0x555555770755 in loop_thread /opt/vcpkg/buildtrees/libhv/src/v1.3.2-990dc4029c.clean/http/server/HttpServer.cpp:158
    #34 0x7ffff6e15559  (/usr/lib/libc.so.6+0x8b559) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
    #35 0x7ffff6e92a3b  (/usr/lib/libc.so.6+0x108a3b) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)

[fnMrRice]

I think the problem is that req was not parsed before calling Dump() in HttpHandler::sendProxyRequest()

[fnMrRice]

possible fix:

diff --git a/http/server/HttpHandler.cpp b/http/server/HttpHandler.cpp
index 1a693b4..f02d360 100644
--- a/http/server/HttpHandler.cpp
+++ b/http/server/HttpHandler.cpp
@@ -1082,7 +1082,7 @@ EOF
     req->headers["Connection"] = keepalive ? "keep-alive" : "close";
     req->headers["X-Real-IP"] = ip;
     // NOTE: send head + received body
-    std::string msg = req->Dump(true, true);
+    std::string msg = req->Dump(true, false) + req->body;
     // printf("%s\n", msg.c_str());
     req->Reset();
 

[ithewei]

Try this:

diff --git a/http/HttpMessage.h b/http/HttpMessage.h
index cc779c3..17ee731 100644
--- a/http/HttpMessage.h
+++ b/http/HttpMessage.h
@@ -297,6 +297,7 @@ public:
     void* Content() {
         if (content == NULL && body.size() != 0) {
             content = (void*)body.data();
+            content_length = body.size();
         }
         return content;
     }

[fnMrRice]

Try this:

diff --git a/http/HttpMessage.h b/http/HttpMessage.h
index cc779c3..17ee731 100644
--- a/http/HttpMessage.h
+++ b/http/HttpMessage.h
@@ -297,6 +297,7 @@ public:
     void* Content() {
         if (content == NULL && body.size() != 0) {
             content = (void*)body.data();
+            content_length = body.size();
         }
         return content;
     }

It solved the buffer overflow, but the body was incorrect

for multipart/form-data,
here is the data sent by browser:

-----------------------------5125518413293710885640709111
Content-Disposition: form-data; name="index"

1
-----------------------------5125518413293710885640709111
Content-Disposition: form-data; name="name"

root
-----------------------------5125518413293710885640709111--

here is the data forwarded by libhv:

-----------------------------5125518413293710885640709111--
-----------------------------5125518413293710885640709111
Content-Disposition: form-data; name="index"

1
-----------------------------5125518413293710885640709111
Content-Disposition: form-data; name="name"

root

for application/json, body would be appended by a string "null" like this:
null{"index":1, .....}

[ithewei]

I know why. When the http header is received, a proxy connection will be made. If it is connected immediately, req.body will be empty at this time, but DumpBody will automatically generate the body based on the Content-Type header, resulting in this kind of bad case, your fix + my fix should be a relatively complete solution. Can you submit a PR to fix it?