rapidjson icon indicating copy to clipboard operation
rapidjson copied to clipboard

Fix possible null-pointer dereference

Open ljavorsk opened this issue 4 years ago • 3 comments

Check for the schemaDocument variable validity before dereferencing it

This line indicates, that the variable 'schemaDocument' could be null: https://github.com/Tencent/rapidjson/blob/master/include/rapidjson/schema.h#L474

It means that this should be checked always before dereferencing the variable.

This patch is editable. However, I don't have much experience with C++ please correct me if needed.

I have a full covscan log ready for you, if it's safe to paste it here let me know and I will do that.

ljavorsk avatar Oct 13 '20 09:10 ljavorsk

CLA assistant check
All committers have signed the CLA.

tencent-adm avatar Oct 13 '20 09:10 tencent-adm

C++ does not permit this syntax. Need another if.

/home/travis/build/Tencent/rapidjson/include/rapidjson/schema.h:480:70: error: cannot convert ‘bool’ to ‘const ValueType* {aka const rapidjson::GenericValue<rapidjson::UTF8<> >*}’ in initialization
         if (const ValueType* v = GetMember(value, GetNotString()) && schemaDocument) {

miloyip avatar Dec 23 '20 01:12 miloyip

Thanks for trying to address an unsafe code path. However your patches are not going to fix any possible bug because the member initialization list for the Scheme constructor also dereferences the schemaDocument argument in a way that cannot trivially be null-checked. I strongly recommend that you rewrite the constructor to take schemaDocument as a reference such as Schema(SchemaDocumentType & schemaDocument, ...).

JeremyAgost avatar Feb 23 '21 22:02 JeremyAgost