-
Notifications
You must be signed in to change notification settings - Fork 0
Security Audit: Core Library Review (92 files) for CodeRabbit #2
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
base: review-base
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,125 @@ | ||||||||||||||||||||||||||||||||||
| #ifndef _MACARON_BASE64_H_ | ||||||||||||||||||||||||||||||||||
| #define _MACARON_BASE64_H_ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * The MIT License (MIT) | ||||||||||||||||||||||||||||||||||
| * Copyright (c) 2016 tomykaira | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Permission is hereby granted, free of charge, to any person obtaining | ||||||||||||||||||||||||||||||||||
| * a copy of this software and associated documentation files (the | ||||||||||||||||||||||||||||||||||
| * "Software"), to deal in the Software without restriction, including | ||||||||||||||||||||||||||||||||||
| * without limitation the rights to use, copy, modify, merge, publish, | ||||||||||||||||||||||||||||||||||
| * distribute, sublicense, and/or sell copies of the Software, and to | ||||||||||||||||||||||||||||||||||
| * permit persons to whom the Software is furnished to do so, subject to | ||||||||||||||||||||||||||||||||||
| * the following conditions: | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * The above copyright notice and this permission notice shall be | ||||||||||||||||||||||||||||||||||
| * included in all copies or substantial portions of the Software. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, | ||||||||||||||||||||||||||||||||||
| * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF | ||||||||||||||||||||||||||||||||||
| * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND | ||||||||||||||||||||||||||||||||||
| * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE | ||||||||||||||||||||||||||||||||||
| * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION | ||||||||||||||||||||||||||||||||||
| * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION | ||||||||||||||||||||||||||||||||||
| * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #include <cstdint> | ||||||||||||||||||||||||||||||||||
| #include <string> | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| namespace macaron { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| class Base64 { | ||||||||||||||||||||||||||||||||||
| public: | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| static std::string Encode(const std::string data) { | ||||||||||||||||||||||||||||||||||
| static constexpr char sEncodingTable[] = { | ||||||||||||||||||||||||||||||||||
| 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', | ||||||||||||||||||||||||||||||||||
| 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', | ||||||||||||||||||||||||||||||||||
| 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', | ||||||||||||||||||||||||||||||||||
| 'Y', 'Z', 'a', 'b', 'c', 'd', 'e', 'f', | ||||||||||||||||||||||||||||||||||
| 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', | ||||||||||||||||||||||||||||||||||
| 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', | ||||||||||||||||||||||||||||||||||
| 'w', 'x', 'y', 'z', '0', '1', '2', '3', | ||||||||||||||||||||||||||||||||||
| '4', '5', '6', '7', '8', '9', '+', '/' | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| size_t in_len = data.size(); | ||||||||||||||||||||||||||||||||||
| size_t out_len = 4 * ((in_len + 2) / 3); | ||||||||||||||||||||||||||||||||||
| std::string ret(out_len, '\0'); | ||||||||||||||||||||||||||||||||||
| size_t i; | ||||||||||||||||||||||||||||||||||
| char *p = const_cast<char*>(ret.c_str()); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| for (i = 0; i < in_len - 2; i += 3) { | ||||||||||||||||||||||||||||||||||
| *p++ = sEncodingTable[(data[i] >> 2) & 0x3F]; | ||||||||||||||||||||||||||||||||||
| *p++ = sEncodingTable[((data[i] & 0x3) << 4) | ((int) (data[i + 1] & 0xF0) >> 4)]; | ||||||||||||||||||||||||||||||||||
| *p++ = sEncodingTable[((data[i + 1] & 0xF) << 2) | ((int) (data[i + 2] & 0xC0) >> 6)]; | ||||||||||||||||||||||||||||||||||
| *p++ = sEncodingTable[data[i + 2] & 0x3F]; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+54
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Integer underflow causes out-of-bounds access on small inputs. The loop condition 🐛 Proposed fix to prevent underflow- for (i = 0; i < in_len - 2; i += 3) {
+ for (i = 0; in_len >= 3 && i < in_len - 2; i += 3) {Or restructure to check 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| if (i < in_len) { | ||||||||||||||||||||||||||||||||||
| *p++ = sEncodingTable[(data[i] >> 2) & 0x3F]; | ||||||||||||||||||||||||||||||||||
| if (i == (in_len - 1)) { | ||||||||||||||||||||||||||||||||||
| *p++ = sEncodingTable[((data[i] & 0x3) << 4)]; | ||||||||||||||||||||||||||||||||||
| *p++ = '='; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| else { | ||||||||||||||||||||||||||||||||||
| *p++ = sEncodingTable[((data[i] & 0x3) << 4) | ((int) (data[i + 1] & 0xF0) >> 4)]; | ||||||||||||||||||||||||||||||||||
| *p++ = sEncodingTable[((data[i + 1] & 0xF) << 2)]; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| *p++ = '='; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return ret; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| static std::string Decode(const std::string& input, std::string& out) { | ||||||||||||||||||||||||||||||||||
| static constexpr unsigned char kDecodingTable[] = { | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 62, 64, 64, 64, 63, | ||||||||||||||||||||||||||||||||||
| 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, | ||||||||||||||||||||||||||||||||||
| 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, | ||||||||||||||||||||||||||||||||||
| 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, | ||||||||||||||||||||||||||||||||||
| 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64 | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| size_t in_len = input.size(); | ||||||||||||||||||||||||||||||||||
| if (in_len % 4 != 0) return "Input data size is not a multiple of 4"; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| size_t out_len = in_len / 4 * 3; | ||||||||||||||||||||||||||||||||||
| if (input[in_len - 1] == '=') out_len--; | ||||||||||||||||||||||||||||||||||
| if (input[in_len - 2] == '=') out_len--; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+96
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out-of-bounds read on empty input. When 🐛 Proposed fix to handle empty input size_t in_len = input.size();
+ if (in_len == 0) {
+ out.clear();
+ return "";
+ }
if (in_len % 4 != 0) return "Input data size is not a multiple of 4";📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| out.resize(out_len); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| for (size_t i = 0, j = 0; i < in_len;) { | ||||||||||||||||||||||||||||||||||
| uint32_t a = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<int>(input[i++])]; | ||||||||||||||||||||||||||||||||||
| uint32_t b = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<int>(input[i++])]; | ||||||||||||||||||||||||||||||||||
| uint32_t c = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<int>(input[i++])]; | ||||||||||||||||||||||||||||||||||
| uint32_t d = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<int>(input[i++])]; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+106
to
+109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Signed char causes negative array index — out-of-bounds access.
🐛 Proposed fix to ensure unsigned indexing- uint32_t a = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<int>(input[i++])];
- uint32_t b = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<int>(input[i++])];
- uint32_t c = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<int>(input[i++])];
- uint32_t d = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<int>(input[i++])];
+ uint32_t a = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<unsigned char>(input[i++])];
+ uint32_t b = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<unsigned char>(input[i++])];
+ uint32_t c = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<unsigned char>(input[i++])];
+ uint32_t d = input[i] == '=' ? 0 & i++ : kDecodingTable[static_cast<unsigned char>(input[i++])];📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| uint32_t triple = (a << 3 * 6) + (b << 2 * 6) + (c << 1 * 6) + (d << 0 * 6); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (j < out_len) out[j++] = (triple >> 2 * 8) & 0xFF; | ||||||||||||||||||||||||||||||||||
| if (j < out_len) out[j++] = (triple >> 1 * 8) & 0xFF; | ||||||||||||||||||||||||||||||||||
| if (j < out_len) out[j++] = (triple >> 0 * 8) & 0xFF; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return ""; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #endif /* _MACARON_BASE64_H_ */ | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| /* | ||
| * IXBench.cpp | ||
| * Author: Benjamin Sergeant | ||
| * Copyright (c) 2017-2020 Machine Zone, Inc. All rights reserved. | ||
| */ | ||
|
|
||
| #include "IXBench.h" | ||
|
|
||
| #include <iostream> | ||
|
|
||
| namespace ix | ||
| { | ||
| Bench::Bench(const std::string& description) | ||
| : _description(description) | ||
| { | ||
| reset(); | ||
| } | ||
|
|
||
| Bench::~Bench() | ||
| { | ||
| if (!_reported) | ||
| { | ||
| report(); | ||
| } | ||
| } | ||
|
|
||
| void Bench::reset() | ||
| { | ||
| _start = std::chrono::high_resolution_clock::now(); | ||
| _reported = false; | ||
| } | ||
|
|
||
| void Bench::report() | ||
| { | ||
| auto now = std::chrono::high_resolution_clock::now(); | ||
| auto microseconds = std::chrono::duration_cast<std::chrono::microseconds>(now - _start); | ||
|
|
||
| _duration = microseconds.count(); | ||
| std::cerr << _description << " completed in " << _duration << " us" << std::endl; | ||
|
|
||
| setReported(); | ||
| } | ||
|
|
||
| void Bench::record() | ||
| { | ||
| auto now = std::chrono::high_resolution_clock::now(); | ||
| auto microseconds = std::chrono::duration_cast<std::chrono::microseconds>(now - _start); | ||
|
|
||
| _duration = microseconds.count(); | ||
| } | ||
|
|
||
| void Bench::setReported() | ||
| { | ||
| _reported = true; | ||
| } | ||
|
|
||
| uint64_t Bench::getDuration() const | ||
| { | ||
| return _duration; | ||
| } | ||
| } // namespace ix |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /* | ||
| * IXBench.h | ||
| * Author: Benjamin Sergeant | ||
| * Copyright (c) 2017-2020 Machine Zone, Inc. All rights reserved. | ||
| */ | ||
| #pragma once | ||
|
|
||
| #include <chrono> | ||
| #include <cstdint> | ||
| #include <string> | ||
|
|
||
| namespace ix | ||
| { | ||
| class Bench | ||
| { | ||
| public: | ||
| Bench(const std::string& description); | ||
| ~Bench(); | ||
|
|
||
| void reset(); | ||
| void record(); | ||
| void report(); | ||
| void setReported(); | ||
| uint64_t getDuration() const; | ||
|
|
||
| private: | ||
| std::string _description; | ||
| std::chrono::time_point<std::chrono::high_resolution_clock> _start; | ||
| uint64_t _duration; | ||
| bool _reported; | ||
| }; | ||
| } // namespace ix |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| /* | ||
| * IXCancellationRequest.cpp | ||
| * Author: Benjamin Sergeant | ||
| * Copyright (c) 2019 Machine Zone, Inc. All rights reserved. | ||
| */ | ||
|
|
||
| #include "IXCancellationRequest.h" | ||
|
|
||
| #include <cassert> | ||
| #include <chrono> | ||
|
|
||
| namespace ix | ||
| { | ||
| CancellationRequest makeCancellationRequestWithTimeout( | ||
| int secs, std::atomic<bool>& requestInitCancellation) | ||
| { | ||
| assert(secs > 0); | ||
|
|
||
| auto start = std::chrono::system_clock::now(); | ||
| auto timeout = std::chrono::seconds(secs); | ||
|
|
||
| auto isCancellationRequested = [&requestInitCancellation, start, timeout]() -> bool { | ||
| // Was an explicit cancellation requested ? | ||
| if (requestInitCancellation) return true; | ||
|
|
||
| auto now = std::chrono::system_clock::now(); | ||
| if ((now - start) > timeout) return true; | ||
|
|
||
| // No cancellation request | ||
| return false; | ||
| }; | ||
|
|
||
| return isCancellationRequested; | ||
| } | ||
| } // namespace ix |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| /* | ||
| * IXCancellationRequest.h | ||
| * Author: Benjamin Sergeant | ||
| * Copyright (c) 2018 Machine Zone, Inc. All rights reserved. | ||
| */ | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <atomic> | ||
| #include <functional> | ||
|
|
||
| namespace ix | ||
| { | ||
| using CancellationRequest = std::function<bool()>; | ||
|
|
||
| CancellationRequest makeCancellationRequestWithTimeout( | ||
| int seconds, std::atomic<bool>& requestInitCancellation); | ||
| } // namespace ix |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| /* | ||
| * IXConnectionState.cpp | ||
| * Author: Benjamin Sergeant | ||
| * Copyright (c) 2019 Machine Zone, Inc. All rights reserved. | ||
| */ | ||
|
|
||
| #include "IXConnectionState.h" | ||
|
|
||
| namespace ix | ||
| { | ||
| std::atomic<uint64_t> ConnectionState::_globalId(0); | ||
|
|
||
| ConnectionState::ConnectionState() | ||
| : _terminated(false) | ||
| { | ||
| computeId(); | ||
| } | ||
|
|
||
| void ConnectionState::computeId() | ||
| { | ||
| _id = std::to_string(_globalId++); | ||
| } | ||
|
|
||
| const std::string& ConnectionState::getId() const | ||
| { | ||
| return _id; | ||
| } | ||
|
|
||
| std::shared_ptr<ConnectionState> ConnectionState::createConnectionState() | ||
| { | ||
| return std::make_shared<ConnectionState>(); | ||
| } | ||
|
|
||
| void ConnectionState::setOnSetTerminatedCallback(const OnSetTerminatedCallback& callback) | ||
| { | ||
| _onSetTerminatedCallback = callback; | ||
| } | ||
|
|
||
| bool ConnectionState::isTerminated() const | ||
| { | ||
| return _terminated; | ||
| } | ||
|
|
||
| void ConnectionState::setTerminated() | ||
| { | ||
| _terminated = true; | ||
|
|
||
| if (_onSetTerminatedCallback) | ||
| { | ||
| _onSetTerminatedCallback(); | ||
| } | ||
| } | ||
|
|
||
| const std::string& ConnectionState::getRemoteIp() | ||
| { | ||
| return _remoteIp; | ||
| } | ||
|
|
||
| int ConnectionState::getRemotePort() | ||
| { | ||
| return _remotePort; | ||
| } | ||
|
|
||
| void ConnectionState::setRemoteIp(const std::string& remoteIp) | ||
| { | ||
| _remoteIp = remoteIp; | ||
| } | ||
|
|
||
| void ConnectionState::setRemotePort(int remotePort) | ||
| { | ||
| _remotePort = remotePort; | ||
| } | ||
| } // namespace ix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undefined behavior: modifying string through
c_str().const_cast<char*>(ret.c_str())and then writing throughpis undefined behavior. Thec_str()method returns a pointer to a read-only buffer, and modifying it violates the C++ standard.🐛 Proposed fix using legal mutable access
Alternatively in C++17+:
char *p = ret.data();📝 Committable suggestion
🤖 Prompt for AI Agents