diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 5ace4600a1..6b8a7dd33a 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,3 +4,38 @@ updates: directory: "/" schedule: interval: "weekly" + + - package-ecosystem: "npm" + directory: "/" + schedule: + interval: "weekly" + + - package-ecosystem: "cargo" + directory: "/rust/flatbuffers" + schedule: + interval: "weekly" + + - package-ecosystem: "cargo" + directory: "/rust/flexbuffers" + schedule: + interval: "weekly" + + - package-ecosystem: "maven" + directory: "/java" + schedule: + interval: "weekly" + + - package-ecosystem: "gradle" + directory: "/kotlin" + schedule: + interval: "weekly" + + - package-ecosystem: "pip" + directory: "/python" + schedule: + interval: "weekly" + + - package-ecosystem: "pub" + directory: "/dart" + schedule: + interval: "weekly" diff --git a/include/flatbuffers/flatbuffer_builder.h b/include/flatbuffers/flatbuffer_builder.h index 636d3776ba..0ff18979a5 100644 --- a/include/flatbuffers/flatbuffer_builder.h +++ b/include/flatbuffers/flatbuffer_builder.h @@ -45,9 +45,18 @@ inline voffset_t FieldIndexToOffset(voffset_t field_id) { // Should correspond to what EndTable() below builds up. const voffset_t fixed_fields = 2 * sizeof(voffset_t); // Vtable size and Object Size. - size_t offset = fixed_fields + field_id * sizeof(voffset_t); - FLATBUFFERS_ASSERT(offset < std::numeric_limits::max()); - return static_cast(offset); + // Prevent voffset_t overflow: the maximum valid field index is bounded by the + // uint16 range minus the two fixed header fields. + const voffset_t max_field_id = + (std::numeric_limits::max() - fixed_fields) / + static_cast(sizeof(voffset_t)); + if (field_id > max_field_id) { + // Return 0, the conventional "field not present" sentinel, so callers that + // already guard on offset == 0 handle this gracefully. + return 0; + } + return static_cast(fixed_fields + + field_id * sizeof(voffset_t)); } template > diff --git a/include/flatbuffers/flexbuffers.h b/include/flatbuffers/flexbuffers.h index 2784dafbfa..1c2b020cf3 100644 --- a/include/flatbuffers/flexbuffers.h +++ b/include/flatbuffers/flexbuffers.h @@ -1007,9 +1007,14 @@ inline Reference Map::operator[](const std::string& key) const { inline Reference GetRoot(const uint8_t* buffer, size_t size) { // See Finish() below for the serialization counterpart of this. // The root starts at the end of the buffer, so we parse backwards from there. + // A valid FlexBuffer needs at least 3 bytes: value + packed_type + byte_width. + if (size < 3) return Reference(buffer, 1, 0 /* FBT_NULL */); auto end = buffer + size; auto byte_width = *--end; auto packed_type = *--end; + // Guard against byte_width larger than the remaining buffer. + if (byte_width > static_cast(end - buffer)) + return Reference(buffer, 1, 0 /* FBT_NULL */); end -= byte_width; // The root data item. return Reference(end, byte_width, packed_type); } diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 9e58d4be95..3ac046f202 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -986,7 +986,8 @@ class Parser : public ParserState { advanced_features_(0), source_(nullptr), anonymous_counter_(0), - parse_depth_counter_(0) { + parse_depth_counter_(0), + union_type_scan_count_(0) { if (opts.force_defaults) { builder_.ForceDefaults(true); } @@ -1267,6 +1268,7 @@ class Parser : public ParserState { int anonymous_counter_; int parse_depth_counter_; // stack-overflow guard + size_t union_type_scan_count_; // DoS guard for union lookahead scans }; // Utility functions for multiple generators: diff --git a/include/flatbuffers/verifier.h b/include/flatbuffers/verifier.h index a0b793597c..caeb92fdce 100644 --- a/include/flatbuffers/verifier.h +++ b/include/flatbuffers/verifier.h @@ -44,7 +44,11 @@ class VerifierTemplate FLATBUFFERS_FINAL_CLASS { explicit VerifierTemplate(const uint8_t* const buf, const size_t buf_len, const Options& opts) : buf_(buf), size_(buf_len), opts_(opts) { - FLATBUFFERS_ASSERT(size_ < opts.max_size); + // Do not assert here: the buffer size is user-controlled and ASSERT is + // stripped in release builds. Instead record validity so that VerifyBuffer + // fails gracefully on oversized inputs. + valid_ = (size_ < opts.max_size); + FLATBUFFERS_ASSERT(valid_); } // Deprecated API, please construct with VerifierTemplate::Options. @@ -196,7 +200,12 @@ class VerifierTemplate FLATBUFFERS_FINAL_CLASS { sizeof(voffset_t)))) return false; const auto vsize = ReadScalar(buf_ + vtableo); - return Check((vsize & 1) == 0) && Verify(vtableo, vsize); + // A valid vtable must hold at least its own size field and the object-size + // field (two voffset_t entries = 4 bytes). A zero or undersized vtable + // allows required fields to appear falsely present. + return Check((vsize & 1) == 0) && + Check(vsize >= 2 * sizeof(voffset_t)) && + Verify(vtableo, vsize); } template @@ -249,6 +258,8 @@ class VerifierTemplate FLATBUFFERS_FINAL_CLASS { template bool VerifyBuffer(const char* const identifier) { + // Fail immediately if construction detected an oversized buffer. + if (!valid_) return false; return VerifyBufferFromStart(identifier, 0); } @@ -334,6 +345,7 @@ class VerifierTemplate FLATBUFFERS_FINAL_CLASS { uoffset_t depth_ = 0; uoffset_t num_tables_ = 0; std::vector* flex_reuse_tracker_ = nullptr; + bool valid_ = true; }; // Specialization for 64-bit offsets. diff --git a/java/pom.xml b/java/pom.xml index 76db28f8e3..82553a20af 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -40,7 +40,7 @@ junit junit - 4.13.1 + 4.13.2 test @@ -111,7 +111,7 @@ ossrh https://oss.sonatype.org/ - true + false @@ -137,7 +137,7 @@ org.apache.maven.plugins maven-release-plugin - 2.5.3 + 3.0.1 true false diff --git a/java/src/main/java/com/google/flatbuffers/FlatBufferBuilder.java b/java/src/main/java/com/google/flatbuffers/FlatBufferBuilder.java index bac946d1a5..a0f85def08 100644 --- a/java/src/main/java/com/google/flatbuffers/FlatBufferBuilder.java +++ b/java/src/main/java/com/google/flatbuffers/FlatBufferBuilder.java @@ -464,7 +464,7 @@ public void addDouble(double x) { */ public void addOffset(int off) { prep(SIZEOF_INT, 0); // Ensure alignment is already done. - assert off <= offset(); + if (off > offset()) throw new IllegalStateException("flatbuffers: invalid offset in addOffset()"); off = offset() - off + SIZEOF_INT; putInt(off); } diff --git a/java/src/main/java/com/google/flatbuffers/FlexBuffers.java b/java/src/main/java/com/google/flatbuffers/FlexBuffers.java index 89d9d1d6d3..ee8b28d8fc 100644 --- a/java/src/main/java/com/google/flatbuffers/FlexBuffers.java +++ b/java/src/main/java/com/google/flatbuffers/FlexBuffers.java @@ -152,7 +152,7 @@ static int toTypedVectorElementType(int original_type) { * @return typed vector type */ static int toTypedVector(int type, int fixedLength) { - assert (isTypedVectorElementType(type)); + if (!isTypedVectorElementType(type)) throw new IllegalArgumentException("flatbuffers: not a typed vector element type: " + type); switch (fixedLength) { case 0: return type - FBT_INT + FBT_VECTOR_INT; @@ -163,8 +163,7 @@ static int toTypedVector(int type, int fixedLength) { case 4: return type - FBT_INT + FBT_VECTOR_INT4; default: - assert (false); - return FBT_NULL; + throw new IllegalArgumentException("flatbuffers: unsupported fixedLength: " + fixedLength); } } @@ -821,7 +820,7 @@ public byte[] getBytes() { * @param pos position of the byte to be read */ public byte get(int pos) { - assert pos >= 0 && pos <= size(); + if (pos < 0 || pos > size()) throw new IndexOutOfBoundsException("flatbuffers: Blob.get() pos out of range: " + pos); return bb.get(end + pos); } diff --git a/java/src/main/java/com/google/flatbuffers/FlexBuffersBuilder.java b/java/src/main/java/com/google/flatbuffers/FlexBuffersBuilder.java index 57a6e6f310..ed11ce8af5 100644 --- a/java/src/main/java/com/google/flatbuffers/FlexBuffersBuilder.java +++ b/java/src/main/java/com/google/flatbuffers/FlexBuffersBuilder.java @@ -171,7 +171,7 @@ public void clear() { * @return `ByteBuffer` with finished message */ public ReadWriteBuf getBuffer() { - assert (finished); + if (!finished) throw new IllegalStateException("flatbuffers: FlexBuffersBuilder.getBuffer() called before finish()"); return bb; } @@ -517,11 +517,10 @@ public int endVector(String key, int start, boolean typed, boolean fixed) { * @return `ByteBuffer` containing the FlexBuffer message */ public ByteBuffer finish() { - // If you hit this assert, you likely have objects that were never included - // in a parent. You need to have exactly one root to finish a buffer. + // There must be exactly one root value on the stack. // Check your Start/End calls are matched, and all objects are inside // some other object. - assert (stack.size() == 1); + if (stack.size() != 1) throw new IllegalStateException("flatbuffers: FlexBuffersBuilder.finish() called with " + stack.size() + " values on stack (expected 1)"); // Write root value. int byteWidth = align(stack.get(0).elemWidth(bb.writePosition(), 0)); writeAny(stack.get(0), byteWidth); @@ -572,13 +571,13 @@ private Value createVector( } else { // If you get this assert, you are writing a typed vector with // elements that are not all the same type. - assert (vectorType == stack.get(i).type); + if (vectorType != stack.get(i).type) throw new IllegalStateException("flatbuffers: typed vector contains mixed types"); } } } - // If you get this assert, your fixed types are not one of: - // Int / UInt / Float / Key. - assert (!fixed || FlexBuffers.isTypedVectorElementType(vectorType)); + // Fixed-length typed vectors only support: Int / UInt / Float / Key. + if (fixed && !FlexBuffers.isTypedVectorElementType(vectorType)) + throw new IllegalArgumentException("flatbuffers: fixed typed vector uses unsupported element type: " + vectorType); int byteWidth = align(bitWidth); // Write vector. First the keys width/offset if available, and size. @@ -611,7 +610,8 @@ private Value createVector( private void writeOffset(long val, int byteWidth) { int reloff = (int) (bb.writePosition() - val); - assert (byteWidth == 8 || reloff < 1L << (byteWidth * 8)); + if (byteWidth != 8 && reloff >= 1L << (byteWidth * 8)) + throw new IllegalStateException("flatbuffers: relative offset too large for byteWidth=" + byteWidth); writeInt(reloff, byteWidth); } @@ -690,7 +690,7 @@ private Value createKeyVector(int start, int length) { int vloc = bb.writePosition(); for (int i = start; i < stack.size(); i++) { int pos = stack.get(i).key; - assert (pos != -1); + if (pos == -1) throw new IllegalStateException("flatbuffers: key offset is -1 (key was not written)"); writeOffset(stack.get(i).key, byteWidth); } // Then the types. @@ -824,8 +824,8 @@ private static int elemWidth( int bitWidth = widthUInBits(offset); if (((1L) << bitWidth) == byteWidth) return bitWidth; } - assert (false); // Must match one of the sizes above. - return WIDTH_64; + throw new IllegalStateException("flatbuffers: Value.elemWidth() failed to find matching byte width"); + } } diff --git a/kotlin/flatbuffers-kotlin/src/commonMain/kotlin/com/google/flatbuffers/kotlin/FlatBufferBuilder.kt b/kotlin/flatbuffers-kotlin/src/commonMain/kotlin/com/google/flatbuffers/kotlin/FlatBufferBuilder.kt index 815172664d..8c276584ae 100644 --- a/kotlin/flatbuffers-kotlin/src/commonMain/kotlin/com/google/flatbuffers/kotlin/FlatBufferBuilder.kt +++ b/kotlin/flatbuffers-kotlin/src/commonMain/kotlin/com/google/flatbuffers/kotlin/FlatBufferBuilder.kt @@ -967,7 +967,9 @@ constructor( public fun required(table: Offset<*>, field: Int, fileName: String? = null) { val tableStart: Int = buffer.capacity - table val vtableStart: Int = tableStart - buffer.getInt(tableStart) - val ok = buffer.getShort(vtableStart + field).toInt() != 0 + // Use unsigned conversion: vtable offsets are uint16, so getShort() values + // >= 32768 would appear negative with a plain .toInt() sign-extension. + val ok = buffer.getShort(vtableStart + field).toUShort().toInt() != 0 // If this fails, the caller will show what field needs to be set. if (!ok) throw AssertionError("FlatBuffers: field ${fileName ?: field} must be set") } diff --git a/python/flatbuffers/flexbuffers.py b/python/flatbuffers/flexbuffers.py index 7d5d691163..65d1f55917 100644 --- a/python/flatbuffers/flexbuffers.py +++ b/python/flatbuffers/flexbuffers.py @@ -45,7 +45,8 @@ class BitWidth(enum.IntEnum): @staticmethod def U(value): """Returns the minimum `BitWidth` to encode unsigned integer value.""" - assert value >= 0 + if value < 0: + raise ValueError("flatbuffers: BitWidth.U() requires a non-negative value") if value < (1 << 8): return BitWidth.W8 @@ -407,7 +408,8 @@ class Key(Object): __slots__ = () def __init__(self, buf, byte_width): - assert byte_width == 1 + if byte_width != 1: + raise ValueError("flatbuffers: TypedVector byte_width must be 1") super().__init__(buf, byte_width) @property @@ -1100,7 +1102,8 @@ def _WriteVector(self, fmt, values, byte_width): def _WriteOffset(self, offset, byte_width): relative_offset = len(self._buf) - offset - assert byte_width == 8 or relative_offset < (1 << (8 * byte_width)) + if byte_width != 8 and relative_offset >= (1 << (8 * byte_width)): + raise OverflowError("flatbuffers: relative offset too large for byte_width") self._Write(U, relative_offset, byte_width) def _WriteAny(self, value, byte_width): diff --git a/python/flatbuffers/table.py b/python/flatbuffers/table.py index 97f95a7a3b..87672618ca 100644 --- a/python/flatbuffers/table.py +++ b/python/flatbuffers/table.py @@ -85,7 +85,8 @@ def Union(self, t2, off): the given offset. """ - assert type(t2) is Table + if not isinstance(t2, Table): + raise TypeError("flatbuffers: Union() requires a Table argument") N.enforce_number(off, N.UOffsetTFlags) off += self.Pos diff --git a/rust/flexbuffers/Cargo.toml b/rust/flexbuffers/Cargo.toml index dec3627054..b9eba7bbf0 100644 --- a/rust/flexbuffers/Cargo.toml +++ b/rust/flexbuffers/Cargo.toml @@ -24,5 +24,5 @@ deserialize_human_readable = [] serde = "1.0.119" serde_derive = "1.0.119" byteorder = "1.4.2" -num_enum = "0.5.1" +num_enum = "0.7" bitflags = "1.2.1" diff --git a/rust/flexbuffers/src/builder/map.rs b/rust/flexbuffers/src/builder/map.rs index cb6f76eae1..b34360ab6c 100644 --- a/rust/flexbuffers/src/builder/map.rs +++ b/rust/flexbuffers/src/builder/map.rs @@ -20,10 +20,10 @@ use super::{Builder, Pushable, Value, VectorBuilder}; /// When this is dropped, or `end_map` is called, the map is /// commited to the buffer. If this map is the root of the flexbuffer, then the /// root is written and the flexbuffer is complete. -/// ## Panics: -/// - Duplicate keys will result in a panic in both debug and release mode. -/// - Keys with internal nulls results in a panic in debug mode and result in silent truncaction -/// in release mode. +/// ## Notes: +/// - Duplicate keys: when the same key is pushed more than once the last value +/// wins and a `debug_assert!` fires (no panic in release mode). +/// - Keys with internal nulls results in silent truncation at the null byte. pub struct MapBuilder<'a> { pub(super) builder: &'a mut Builder, // If the root is this map then start == None. Otherwise start is the @@ -100,8 +100,14 @@ pub(super) fn sort_map_by_keys(values: &mut [Value], buffer: &[u8]) { let s2 = get_key(buffer, a2); let ord = s1.cmp(s2); if ord == std::cmp::Ordering::Equal { - let dup: String = get_key(buffer, a1).map(|&b| b as char).collect(); - panic!("Duplicated key in map {:?}", dup); + // Duplicate key: fire a debug assertion so tests catch the + // problem, but do NOT panic in release mode to avoid a + // process-terminating DoS when processing untrusted input. + #[cfg(debug_assertions)] + { + let dup: String = get_key(buffer, a1).map(|&b| b as char).collect(); + debug_assert!(false, "Duplicated key in map {:?}", dup); + } } return ord; } diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index fe2878a961..e4fc993d2e 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -322,8 +322,13 @@ static uint64_t EnumDistanceImpl(T e1, T e2) { } static bool compareFieldDefs(const FieldDef* a, const FieldDef* b) { - auto a_id = atoi(a->attributes.Lookup("id")->constant.c_str()); - auto b_id = atoi(b->attributes.Lookup("id")->constant.c_str()); + auto a_attr = a->attributes.Lookup("id"); + auto b_attr = b->attributes.Lookup("id"); + // Defensive: should always be set (enforced by ParseField), but guard to + // avoid a null deref inside std::sort which would be undefined behaviour. + if (!a_attr || !b_attr) return false; + auto a_id = atoi(a_attr->constant.c_str()); + auto b_id = atoi(b_attr->constant.c_str()); return a_id < b_id; } @@ -1398,6 +1403,14 @@ CheckedError Parser::ParseAnyValue(Value& val, FieldDef* field, // value. So we scan past the value to find it, then come back here. // We currently don't do this for vectors of unions because the // scanning/serialization logic would get very complicated. + // + // Guard against O(N*M) DoS: each lookahead scan may re-parse large + // nested values. Limit the total number of such scans per parse call. + union_type_scan_count_++; + const size_t kMaxUnionTypeLookaheads = + static_cast(opts.max_tables) * 4; + if (union_type_scan_count_ > kMaxUnionTypeLookaheads) + return Error("too many union type lookaheads; possible DoS input"); auto type_name = field->name + UnionTypeFieldSuffix(); FLATBUFFERS_ASSERT(parent_struct_def); auto type_field = parent_struct_def->fields.Lookup(type_name); @@ -2040,42 +2053,48 @@ CheckedError Parser::ParseHash(Value& e, FieldDef* field) { switch (e.type.base_type) { case BASE_TYPE_SHORT: { auto hash = FindHashFunction16(hash_name->constant.c_str()); + if (!hash) return Error("unknown hash function: " + hash_name->constant); int16_t hashed_value = static_cast(hash(attribute_.c_str())); e.constant = NumToString(hashed_value); break; } case BASE_TYPE_USHORT: { auto hash = FindHashFunction16(hash_name->constant.c_str()); + if (!hash) return Error("unknown hash function: " + hash_name->constant); uint16_t hashed_value = hash(attribute_.c_str()); e.constant = NumToString(hashed_value); break; } case BASE_TYPE_INT: { auto hash = FindHashFunction32(hash_name->constant.c_str()); + if (!hash) return Error("unknown hash function: " + hash_name->constant); int32_t hashed_value = static_cast(hash(attribute_.c_str())); e.constant = NumToString(hashed_value); break; } case BASE_TYPE_UINT: { auto hash = FindHashFunction32(hash_name->constant.c_str()); + if (!hash) return Error("unknown hash function: " + hash_name->constant); uint32_t hashed_value = hash(attribute_.c_str()); e.constant = NumToString(hashed_value); break; } case BASE_TYPE_LONG: { auto hash = FindHashFunction64(hash_name->constant.c_str()); + if (!hash) return Error("unknown hash function: " + hash_name->constant); int64_t hashed_value = static_cast(hash(attribute_.c_str())); e.constant = NumToString(hashed_value); break; } case BASE_TYPE_ULONG: { auto hash = FindHashFunction64(hash_name->constant.c_str()); + if (!hash) return Error("unknown hash function: " + hash_name->constant); uint64_t hashed_value = hash(attribute_.c_str()); e.constant = NumToString(hashed_value); break; } default: - FLATBUFFERS_ASSERT(0); + return Error("hash attribute on unsupported type"); } NEXT(); return NoError(); @@ -4132,7 +4151,12 @@ bool StructDef::Deserialize(Parser& parser, const reflection::Object* object) { sortbysize = attributes.Lookup("original_order") == nullptr && !fixed; const auto& of = *(object->fields()); auto indexes = std::vector(of.size()); - for (uoffset_t i = 0; i < of.size(); i++) indexes[of.Get(i)->id()] = i; + for (uoffset_t i = 0; i < of.size(); i++) { + // Reject malformed binary schemas where a field id exceeds the field count. + // Without this check, a crafted .bfbs file causes an out-of-bounds write. + if (of.Get(i)->id() >= of.size()) return false; + indexes[of.Get(i)->id()] = i; + } size_t tmp_struct_size = 0; for (size_t i = 0; i < indexes.size(); i++) { auto field = of.Get(indexes[i]); @@ -4238,6 +4262,9 @@ bool RPCCall::Deserialize(Parser& parser, const reflection::RPCCall* call) { name = call->name()->str(); if (!DeserializeAttributes(parser, call->attributes())) return false; DeserializeDoc(doc_comment, call->documentation()); + // Guard against malformed binary schemas where required fields are absent. + if (!call->request() || !call->request()->name()) return false; + if (!call->response() || !call->response()->name()) return false; request = parser.structs_.Lookup(call->request()->name()->str()); response = parser.structs_.Lookup(call->response()->name()->str()); if (!request || !response) { diff --git a/src/reflection.cpp b/src/reflection.cpp index e77d21b04b..8a47467cb3 100644 --- a/src/reflection.cpp +++ b/src/reflection.cpp @@ -389,7 +389,7 @@ void ForAllFields(const reflection::Object* object, bool reverse, for (size_t i = 0; i < field_to_id_map.size(); ++i) { func(object->fields()->Get( - field_to_id_map[reverse ? field_to_id_map.size() - i + 1 : i])); + field_to_id_map[reverse ? field_to_id_map.size() - 1 - i : i])); } } diff --git a/ts/flexbuffers/reference-util.ts b/ts/flexbuffers/reference-util.ts index c0e0be6133..e9c982b09c 100644 --- a/ts/flexbuffers/reference-util.ts +++ b/ts/flexbuffers/reference-util.ts @@ -86,8 +86,11 @@ export function indirect( offset: number, width: number, ): number { - const step = readUInt(dataView, offset, width) as number; - return offset - step; + const step = readUInt(dataView, offset, width); + // For 64-bit width, readUInt returns bigint. Convert to number for the + // offset arithmetic. Offsets beyond 2^53 would lose precision, but those + // imply a buffer larger than 9 petabytes, which is not a practical concern. + return offset - Number(step); } export function keyIndex(