Skip to content

Commit

Permalink
Fix SIGSEGV in openssl_random_pseudo_bytes and xor_strings (#985)
Browse files Browse the repository at this point in the history
SIGSEGV was caused by using wrong string's constructor.
The constructor we used was an optimization for single-character
strings. Such strings are stored in `.rodata`. Writing to these strings'
buffer led to a SIGSEGV
  • Loading branch information
apolyakov authored Apr 27, 2024
1 parent e56cfd3 commit 3d1f228
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 3 deletions.
4 changes: 2 additions & 2 deletions runtime/openssl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ Optional<string> f$openssl_random_pseudo_bytes(int64_t length) {
if (length <= 0 || length > string::max_size()) {
return false;
}
string buffer(static_cast<string::size_type>(length), ' ');
string buffer{static_cast<string::size_type>(length), false};
timeval tv{};
gettimeofday(&tv, nullptr);

Expand All @@ -610,7 +610,7 @@ Optional<string> f$openssl_random_pseudo_bytes(int64_t length) {
if (RAND_bytes(reinterpret_cast<unsigned char *>(buffer.buffer()), static_cast<int32_t>(length)) <= 0) {
return false;
}
return std::move(buffer);
return buffer;
}


Expand Down
4 changes: 4 additions & 0 deletions runtime/string_decl.inl
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ private:
inline void set_size(size_type new_size);

inline static char *create(const char *beg, const char *end);
// IMPORTANT: this function may return read-only strings for n == 0 and n == 1.
// Use it unless you have to manually write something into the buffer.
inline static char *create(size_type req, char c);
inline static char *create(size_type req, bool b);

Expand All @@ -97,6 +99,8 @@ public:
inline string(string &&str) noexcept;
inline string(const char *s, size_type n);
inline explicit string(const char *s);
// IMPORTANT: this constructor may return read-only strings for n == 0 and n == 1.
// Use it unless you have to manually operate with string's internal buffer.
inline string(size_type n, char c);
inline string(size_type n, bool b);
inline explicit string(int64_t i);
Expand Down
2 changes: 1 addition & 1 deletion runtime/string_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2844,7 +2844,7 @@ string f$wordwrap(const string &str, int64_t width, const string &brk, bool cut)

string f$xor_strings(const string &s, const string &t) {
string::size_type length = min(s.size(), t.size());
string result(length, ' ');
string result{length, false};
const char *s_str = s.c_str();
const char *t_str = t.c_str();
char *res_str = result.buffer();
Expand Down
33 changes: 33 additions & 0 deletions tests/phpt/openssl/9_openssl_random_pseudo_bytes.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
@ok
<?php

function is_kphp() {
#ifndef KPHP
return false;
#endif
return true;
}

function test_openssl_random_pseudo_bytes() {
for ($i = -300; $i < 2048; $i += 273) {
$res = "";

if (is_kphp()) {
$res = openssl_random_pseudo_bytes($i);
if ($res === false && $i < 1) {
print("error");
}
} else {
try {
$res = openssl_random_pseudo_bytes($i);
} catch (Exception $e) {
if ($i < 1) {
print("error");
} else {
critical_error("unexpected exception");
}
}
}
var_dump($res);
}
}
24 changes: 24 additions & 0 deletions tests/phpt/string_functions/010_xor_strings.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
@ok
<?php

function is_kphp() {
#ifndef KPHP
return false;
#endif
return true;
}

function test_xor_strings() {
$strings = ["handling and manipulating", "ings, take a look", "ctory Access Protocol", "0000000dwxasd", "1111sadsd11", "a", "-"];

foreach ($strings as $str1) {
foreach($strings as $str2) {
if (is_kphp()) {
$res = xor_strings($str1, $str2);
} else {
$res = $str1 ^ $str2;
}
var_dump($res);
}
}
}

0 comments on commit 3d1f228

Please sign in to comment.