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

Remove possiblity of underflows from loop variables #487

Merged
merged 1 commit into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/workerd/api/encoding.c++
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ Encoding getEncodingForLabel(kj::StringPtr label) {
c == 0x0d /* cr */ ||
c == 0x20 /* sp */;
};
auto start = 0;
size_t start = 0;
auto end = label.size();
while (start < end && isAsciiWhitespace(label[start])) { start++; }
while (end > start && isAsciiWhitespace(label[end - 1])) { end--; }
Expand Down
6 changes: 6 additions & 0 deletions src/workerd/api/form-data.c++
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,12 @@ void FormData::forEach(
auto localParams = KJ_ASSERT_NONNULL(JSG_THIS.tryGetHandle(isolate));

auto context = isolate->GetCurrentContext(); // Needed later for Call().

// On each iteration of the for loop, a JavaScript callback is invokved. If a new
// item is appended to the URLSearchParams within that function, the loop must pick
// it up. Using the classic for (;;) syntax here allows for that. However, this does
// mean that it's possible for a user to trigger an infinite loop here if new items
// are added to the search params unconditionally on each iteration.
for (int i = 0; i < this->data.size(); i++) {
auto& [key, value] = this->data[i];
static constexpr auto ARG_COUNT = 3;
Expand Down
6 changes: 3 additions & 3 deletions src/workerd/api/node/buffer-string-search.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@ class StringSearch : private StringSearchBase {
static inline int CharOccurrence(int* bad_char_occurrence,
Char char_code) {
if (sizeof(Char) == 1) {
return bad_char_occurrence[static_cast<int>(char_code)];
return bad_char_occurrence[static_cast<uint>(char_code)];
}
// Both pattern and subject are UC16. Reduce character to equivalence class.
int equiv_class = char_code % kUC16AlphabetSize;
uint equiv_class = char_code % kUC16AlphabetSize;
return bad_char_occurrence[equiv_class];
}

Expand Down Expand Up @@ -532,7 +532,7 @@ void StringSearch<Char>::PopulateBoyerMooreHorspoolTable() {
}
for (size_t i = start; i < pattern_length - 1; i++) {
Char c = pattern_[i];
int bucket = (sizeof(Char) == 1) ? c : c % AlphabetSize();
uint bucket = (sizeof(Char) == 1) ? c : c % AlphabetSize();
bad_char_occurrence[bucket] = i;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/node/buffer.c++
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ kj::Array<byte> decodeHexTruncated(kj::ArrayPtr<kj::byte> text, bool strict = fa
}
kj::Vector vec = kj::Vector<kj::byte>(text.size() / 2);

for (auto i = 0; i < text.size(); i += 2) {
for (size_t i = 0; i < text.size(); i += 2) {
byte b = 0;
KJ_IF_MAYBE(d1, tryFromHexDigit(text[i])) {
b = *d1 << 4;
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/sockets.c++
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ bool isValidHost(kj::StringPtr host) {
return false;
}

for (int i = 0; i < host.size(); i++) {
for (auto i : kj::indices(host)) {
switch (host[i]) {
case '-':
case '.':
Expand Down
6 changes: 3 additions & 3 deletions src/workerd/api/url-standard.c++
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ void percentEncodeCodepoint(
return;
}
auto len = codepointToUtf8(buf, codepoint);
for (auto n = 0; n < len; n++) {
for (size_t n = 0; n < len; n++) {
builder.add('%');
hexEncode(builder, buf[n]);
}
Expand Down Expand Up @@ -401,7 +401,7 @@ jsg::UsvString percentDecode(jsg::UsvStringPtr input) {
const auto appendAsUtf8 = [&result](auto codepoint) {
kj::byte buf[4];
auto len = codepointToUtf8(buf, codepoint);
for (auto n = 0; n < len; n++) {
for (size_t n = 0; n < len; n++) {
result.add(buf[n]);
}
};
Expand Down Expand Up @@ -2052,7 +2052,7 @@ void URLSearchParams::forEach(
// it up. Using the classic for (;;) syntax here allows for that. However, this does
// mean that it's possible for a user to trigger an infinite loop here if new items
// are added to the search params unconditionally on each iteration.
for (int i = 0; i < list.size(); i++) {
for (size_t i = 0; i < list.size(); i++) {
auto& entry = list[i];
v8::Local<v8::Value> args[3] = {
jsg::v8Str(isolate, entry.value),
Expand Down
7 changes: 6 additions & 1 deletion src/workerd/api/url.c++
Original file line number Diff line number Diff line change
Expand Up @@ -596,8 +596,13 @@ void URLSearchParams::forEach(
// from JavaScript, which means a Headers JS wrapper object must already exist.
auto localParams = KJ_ASSERT_NONNULL(JSG_THIS.tryGetHandle(isolate));

// On each iteration of the for loop, a JavaScript callback is invokved. If a new
// item is appended to the this->url->query within that function, the loop must pick
// it up. Using the classic for (;;) syntax here allows for that. However, this does
// mean that it's possible for a user to trigger an infinite loop here if new items
// are added to the search params unconditionally on each iteration.
auto context = isolate->GetCurrentContext(); // Needed later for Call().
for (int i = 0; i < this->url->query.size(); i++) {
for (size_t i = 0; i < this->url->query.size(); i++) {
auto& [key, value] = this->url->query[i];
static constexpr auto ARG_COUNT = 3;
v8::Local<v8::Value> args[ARG_COUNT] = {
Expand Down
9 changes: 7 additions & 2 deletions src/workerd/api/urlpattern.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,12 @@ jsg::UsvString generatePatternString(kj::ArrayPtr<Part> partList, const CompileO
return false;
};

for (auto n = 0; n < partList.size(); n++) {
// On each iteration of the for loop, a JavaScript callback is invokved. If a new
// item is appended to the partList within that function, the loop must pick
// it up. Using the classic for (;;) syntax here allows for that. However, this does
// mean that it's possible for a user to trigger an infinite loop here if new items
// are added to the search params unconditionally on each iteration.
for (size_t n = 0; n < partList.size(); n++) {
auto& part = partList[n];
Part* previousPart = nullptr;
Part* nextPart = nullptr;
Expand Down Expand Up @@ -1481,7 +1486,7 @@ URLPattern::URLPatternInit parseConstructorString(
}
};

const auto getSafeToken = [&tokenList](auto index) -> Token& {
const auto getSafeToken = [&tokenList](size_t index) -> Token& {
if (index < tokenList.size()) {
return tokenList[index];
}
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/util.c++
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ kj::Maybe<kj::ArrayPtr<const char>> readContentTypeParameter(kj::StringPtr conte

if (leftover[valueStart] == '"') {
// parameter value surrounded by quotes
auto pos = 0;
size_t pos = 0;
auto valueStr = leftover.slice(valueStart + 1);

while(pos < valueStr.size()) {
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/worker.c++
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ static void stopProfiling(v8::CpuProfiler& profiler,v8::Isolate* isolate,
profile.setEndTime(cpuProfile->GetEndTime());

auto nodes = profile.initNodes(allNodes.size());
for (int i=0; i < allNodes.size(); i++) {
for (auto i : kj::indices(allNodes)) {
auto nodeBuilder = nodes[i];
nodeBuilder.setId(allNodes[i]->GetNodeId());

Expand All @@ -798,7 +798,7 @@ static void stopProfiling(v8::CpuProfiler& profiler,v8::Isolate* isolate,
allNodes[i]->GetLineTicks(lineBuffer, hitLineCount);

auto positionTicks = nodeBuilder.initPositionTicks(hitLineCount);
for (int j=0; j < hitLineCount; j++) {
for (uint j=0; j < hitLineCount; j++) {
auto positionTick = positionTicks[j];
positionTick.setLine(lineBuffer[j].line);
positionTick.setTicks(lineBuffer[j].hit_count);
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/jsg/rtti.h
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ struct MembersBuilder {
Structure::Builder structure;
capnp::List<Member>::Builder members;
Builder<Configuration>& rtti;
int index = 0;
uint index = 0;

MembersBuilder(Structure::Builder structure,
capnp::List<Member>::Builder members,
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/jsg/ser.c++
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void Deserializer::init(
deser.SetWireFormatVersion(*version);
}
KJ_IF_MAYBE(arrayBuffers, transferedArrayBuffers) {
for (auto n = 0; n < arrayBuffers->size(); n++) {
for (auto n : kj::indices(*arrayBuffers)) {
deser.TransferArrayBuffer(n,
v8::ArrayBuffer::New(isolate, kj::mv((*arrayBuffers)[n])));
}
Expand Down