chakra-core / chakra-core/ChakraCore

JavascriptArray bugs in numeric propertyId handling

Open
#526 5 comments 0 reactions 0 assignees View on GitHub
Bug Severity: 2
Dominant language
JavaScript
Stars
9.3k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

There is an issue in multiple JavascriptArray methods when handling numeric propertyId. Needs a thorough review of JavascriptArray/ES5Array methods. We rarely hit these code paths because we usually call uint32 index methods. With JSRT, one can create a numeric propertyId and use it to access an array.

E.g., compare 2 methods:

```
BOOL JavascriptArray::DeleteProperty(PropertyId propertyId, PropertyOperationFlags flags)
{
if (propertyId == PropertyIds::length)
{
return false;
}
return DynamicObject::DeleteProperty(propertyId, flags);
}

BOOL JavascriptArray::HasProperty(PropertyId propertyId)
{
if (propertyId == PropertyIds::length)
{
return true;
}

ScriptContext* scriptContext = GetScriptContext();
uint32 index;
if (scriptContext->IsNumericPropertyId(propertyId, &index))
{
return this->HasItem(index);
}

return DynamicObject::HasProperty(propertyId);
}

```

`DeleteProperty` is missing numeric propertyId handling available in `HasProperty`. It delegates to `DynamicObject::DeleteProperty`, which invokes `TypeHandler::DeleteProperty`. But `TypeHandler` methods do not handle array instance with numeric proeprtyId either. They all have following pattern and only deal with object's internal array:

```
// Check numeric propertyRecord only if objectArray available
if (instance->HasObjectArray() && propertyRecord->IsNumeric())
{
return DictionaryTypeHandlerBase::DeleteItem(instance, propertyRecord->GetNumericValue(), propertyOperationFlags);
}

return true;
```

From a glance several JavascriptArray methods have this problem... `IsEnumerable`, `IsConfigurable`... Not sure if issue exists in ES5Array too.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.