Skip to content

Unchecked numeric narrowing (.intValue()/.floatValue()) widespread in OpenCypher procedures package (follow-up to #5919) #5924

Description

@lvca

Summary

Follow-up to the unchecked-numeric-narrowing family (#5900, #5905, #5906, #5919). While fixing #5919 (LIMIT/SKIP, map2json, DOUBLE MIN/MAX), the code review on PR #5921 flagged that the same anti-pattern - narrowing a wider numeric with .intValue()/.floatValue() without a range check - is widespread in the OpenCypher procedures package, which was not part of #5919's scope.

A grep for the pattern turns up 35 files under engine/src/main/java/com/arcadedb/query/opencypher/procedures/ (path/traversal procedures and graph algorithm procedures - PageRank, Louvain, Leiden, Node2Vec, GraphSAGE, KNN, etc.). Representative examples:

// PathExpandConfig.java:103 - apoc.path.expandConfig's "limit" option
final int limit = config.containsKey("limit") ? ((Number) config.get("limit")).intValue() : Integer.MAX_VALUE;

// DbIndexVectorQueryNodes.java:101 - db.index.vector.queryNodes's limit argument
final int limit = args[1] instanceof Number n ? n.intValue() : Integer.parseInt(args[1].toString());

// AlgoNode2Vec.java:103-107 - walksPerNode/windowSize/negSamples/etc. from the config map
final int walksPerNode = config != null && config.get("walksPerNode") instanceof Number n ? n.intValue() : 10;

All of these read a Number out of a Cypher procedure argument list or config map (user-controlled) and narrow it to int with .intValue()/.floatValue() with no bounds check. A caller passing a value outside int range (e.g. a Long bind param, or a numeric literal Cypher parses wider than 32 bits) silently wraps instead of erroring or saturating - the same class of bug as #5919's LIMIT 2147483648 returning 0 rows.

Honest scope, deliberately not claiming each site is independently exploitable: unlike LIMIT, most of these are algorithm tuning knobs (iteration counts, embedding dimensions, walk counts) that are rarely supplied as huge values in practice, so the blast radius per-site is likely lower than #5919's Site A. This issue is filed to get the pattern tracked and triaged as one sweep - per #5919's own framing ("this is now 6+ sites... it wants one codebase-wide sweep, not point fixes") - not to assert each of the 35 sites is a confirmed wrong-result bug the way #5919's three sites were runtime/code-verified.

Explicitly NOT part of this pattern (checked and ruled out, so they don't get swept in by a blind grep-and-fix):

  • gremlin/.../GraphSONImporterFormat.java:272,274 - these narrow based on an explicit g:Int32/g:Float GraphSON type tag, i.e. the source data declares its own width; narrowing to match the declared type is correct, not an unchecked-narrowing bug.
  • gremlin/.../ArcadeIoRegistry.java:67 - narrows a bucket ID to int, which is the bucket ID's actual declared type throughout the schema APIs (RID.create takes an int bucketId), not a wider-to-narrower narrowing.

Suggested fix

Same as #5919: for each site, either widen the comparison (keep as long/double through the computation) or range-check and reject/saturate explicitly (e.g. Math.toIntExact, or the NumberUtils.saturateToInt() helper added in #5921 if silent saturation is the right semantics for that parameter). Given the volume (35 files), this probably wants triage first to bucket sites by:

  1. Parameters that are genuinely just int-sized knobs where Math.toIntExact (fail loud on overflow) is the right fix.
  2. Parameters like limit/maxLevel/maxDepth that mirror SQL's LIMIT/SKIP semantics, where NumberUtils.saturateToInt() is the more consistent fix.

Related: #5900, #5905, #5906, #5919, PR #5921.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions