cjlin1 / liblinear

LIBLINEAR -- A Library for Large Linear Classification
https://www.csie.ntu.edu.tw/~cjlin/liblinear/
BSD 3-Clause "New" or "Revised" License
1.01k stars 342 forks source link

A potential API Misuse bug found in linear.cpp #87

Closed x14ngch3n closed 1 year ago

x14ngch3n commented 1 year ago

In function load_model(), the function first read an integer from model file in line#3501, then it use this value as the allocation size of Malloc() in line#3526.

If the field model->nr_class has been set to a huge value, the following Malloc may probably fail. While the caller doesn't check the return value of Malloc, so it may cause a Null Pointer Dereference.

else if(strcmp(cmd,"nr_class")==0)
{
    fscanf(fp,"%d",&nr_class);
    model_->nr_class=nr_class;
}
...
else if(strcmp(cmd,"label")==0)
{
    int nr_class = model_->nr_class;
    model_->label = Malloc(int,nr_class);
    for(int i=0;i<nr_class;i++)
        fscanf(fp,"%d",&model_->label[i]);
}
KyleLin123456 commented 1 year ago

Thank you for pointing out, do you have any fix to this? Because manipulating batches of malloc() might cause too much cost on both maintenance and performance of the code. If you have any easy fix please let me know, otherwise I think the LIBLINEAR for more than 2^32 instances/features in libsvmtool is the best solution to your problem.

x14ngch3n commented 1 year ago

Sorry for the late reply, I think for this issue, adding an error handling logic (for example, wrapped in the macro Malloc) for malloc failure is a candidate solution.

This is the function Malloc used in CS:APP for your reference: http://www.cs.cmu.edu/afs/cs/academic/class/15213-f01/L7/csapp.c

void *Malloc(size_t size) 
{
    void *p;

    if ((p  = malloc(size)) == NULL)
    unix_error("Malloc error");
    return p;
}
cjlin1 commented 1 year ago

At this moment we do not cover data with more than 2^32 features/labels. If you need to handle such large data, please use "LIBLINEAR for more than 2^32 instances/features (experimental)" at https://www.csie.ntu.edu.tw/~cjlin/libsvmtools/#liblinear_for_more_than_2^32_instances_features_experimental

So we don't plan to do any change here. Thank you again for your comments.