| Message ID | 20241212191228.84504-1-ant.v.moryakov@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | David Oberhollenzer |
| Headers | show |
| Series | [mtd-utils] lib: Fix integer overflow in libiniparser.c | expand |
在 2024/12/13 3:12, Anton Moryakov 写道: > Report of the static analyzer: > Possible integer underflow: left operand is tainted. > An integer underflow may occur due to arithmetic operation (subtraction) between values > { [-2147483648, 2147483647] } and '1', where the first value comes from the expression '(int)strlen(line)' > > Corrections explained: > 1. Added a check to handle empty strings before decrementing the length of the line (`len = strlen(line) - 1`) to prevent integer underflow. > 2. Modified logic toskip empty lines immediately, reducing unnecessary processing.- Ensured safety by validating len >= 0 before accessing line[len] to avoid out-of-bounds access. > 3. Improved robustness when handling multiline values by ensuring valid indices for len.- Added relevant comments to clarify changes and their purpose. > > Triggers found by static analyzer Svace. > > Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com> > > --- > lib/libiniparser.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/lib/libiniparser.c b/lib/libiniparser.c > index a6ddcc7..da980c0 100644 > --- a/lib/libiniparser.c > +++ b/lib/libiniparser.c > @@ -522,9 +522,15 @@ dictionary * iniparser_load(const char * ininame) > > while (fgets(line+last, ASCIILINESZ-last, in)!=NULL) { > lineno++ ; > - len = (int)strlen(line)-1; > + len = (int)strlen(line); Hi, Anton. The 'len' must be greater than 0 if fgets() returns not NULL, the 'line' contains at least one letter '\n'. > + if(len == 0){ > + memset(line, 0, ASCIILINESZ); > + last = 0; > + continue; > + } > + len--; > /* Safety check against buffer overflows */ > - if (line[len]!='\n') { > + if (len < 0 || line[len]!='\n') { > fprintf(stderr, > "iniparser: input line too long in %s (%d)\n", > ininame, > @@ -540,7 +546,7 @@ dictionary * iniparser_load(const char * ininame) > len-- ; > } Here, 'len' could be less than zero. I'm fine with the modification here. > /* Detect multi-line */ > - if (line[len]=='\\') { > + if (len >= 0 && line[len]=='\\') { > /* Multi-line value */ > last=len ; > continue ; >
diff --git a/lib/libiniparser.c b/lib/libiniparser.c index a6ddcc7..da980c0 100644 --- a/lib/libiniparser.c +++ b/lib/libiniparser.c @@ -522,9 +522,15 @@ dictionary * iniparser_load(const char * ininame) while (fgets(line+last, ASCIILINESZ-last, in)!=NULL) { lineno++ ; - len = (int)strlen(line)-1; + len = (int)strlen(line); + if(len == 0){ + memset(line, 0, ASCIILINESZ); + last = 0; + continue; + } + len--; /* Safety check against buffer overflows */ - if (line[len]!='\n') { + if (len < 0 || line[len]!='\n') { fprintf(stderr, "iniparser: input line too long in %s (%d)\n", ininame, @@ -540,7 +546,7 @@ dictionary * iniparser_load(const char * ininame) len-- ; } /* Detect multi-line */ - if (line[len]=='\\') { + if (len >= 0 && line[len]=='\\') { /* Multi-line value */ last=len ; continue ;
Report of the static analyzer: Possible integer underflow: left operand is tainted. An integer underflow may occur due to arithmetic operation (subtraction) between values { [-2147483648, 2147483647] } and '1', where the first value comes from the expression '(int)strlen(line)' Corrections explained: 1. Added a check to handle empty strings before decrementing the length of the line (`len = strlen(line) - 1`) to prevent integer underflow. 2. Modified logic toskip empty lines immediately, reducing unnecessary processing.- Ensured safety by validating len >= 0 before accessing line[len] to avoid out-of-bounds access. 3. Improved robustness when handling multiline values by ensuring valid indices for len.- Added relevant comments to clarify changes and their purpose. Triggers found by static analyzer Svace. Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com> --- lib/libiniparser.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)