Changeset 2fa0237

Timestamp:

Sep 8, 2023, 12:46:55 AM (8 months ago)

Author:

Michael Brooks <mlbrooks@…>

Branches:

master

Children:

553f032f

Parents:

3ee8853

Message:

Fix cstring input length interpretation issue that had a buffer overflow case.

The cases added to the manipulatorsInput test are runnable against an old libcfa build. In this setup, the test fails with an illustration of the bug.

The testing in this commit drives the following inputs through a length-8 buffer.

• 123456
• 123456789

The obviously-missing cases, like 1234567, will be added later.
They will accompany fixes for further bugs not solved yet.

Files:

• libcfa/src/iostream.cfa (modified) (3 diffs)
• tests/io/.expect/manipulatorsInput.arm64.txt (modified) (1 diff)
• tests/io/.expect/manipulatorsInput.x64.txt (modified) (1 diff)
• tests/io/.expect/manipulatorsInput.x86.txt (modified) (1 diff)
• tests/io/.in/manipulatorsInput.txt (modified) (1 diff)
• tests/io/manipulatorsInput.cfa (modified) (1 diff)

libcfa/src/iostream.cfa

@@ +1 @@
+
 //
 // Cforall Version 1.0.0 Copyright (C) 2015 University of Waterloo
@@ -976 +977 @@
                 if ( f.flags.ignore ) { fmtstr[1] = '*'; start += 1; }
                 // no maximum width necessary because text ignored => width is read width
-                if ( f.wd != -1 ) { start += sprintf( &fmtstr[start], "%d", f.wd ); }
+                if ( f.wd != -1 ) {
+                        // wd is buffer bytes available (for input chars + null terminator)
+                        // rwd is count of input chars
+                        int rwd = f.flags.rwd ? f.wd : (f.wd - 1);
+                        start += sprintf( &fmtstr[start], "%d", rwd );
+                }
 
                 if ( ! scanset ) {
@@ -993 +999 @@
                 } // if
 
-                int check = f.wd - 1;
+                int check = f.wd - 2;
                 if ( ! f.flags.rwd ) f.s[check] = '\0';                 // insert sentinel
                 len = fmt( is, fmtstr, f.s );

tests/io/.expect/manipulatorsInput.arm64.txt

@@ +1 @@
+pre1 "123456", canary ok
+pre2a "1234567", exception occurred, canary ok
+pre2b "89", canary ok
 1 yyyyyyyyyyyyyyyyyyyy
 2 abcxxx

tests/io/.expect/manipulatorsInput.x64.txt

@@ +1 @@
+pre1 "123456", canary ok
+pre2a "1234567", exception occurred, canary ok
+pre2b "89", canary ok
 1 yyyyyyyyyyyyyyyyyyyy
 2 abcxxx

tests/io/.expect/manipulatorsInput.x86.txt

@@ +1 @@
+pre1 "123456", canary ok
+pre2a "1234567", exception occurred, canary ok
+pre2b "89", canary ok
 1 yyyyyyyyyyyyyyyyyyyy
 2 abcxxx

tests/io/.in/manipulatorsInput.txt

@@ +1 @@
+123456
+123456789
 abc 
 abc 

tests/io/manipulatorsInput.cfa

@@ -15 +15 @@
 
 int main() {
+        {
+                // Upfront checks to ensure buffer safety.  Once these pass, the simpler `wdi(sizeof(s),s)`
+                // usage, as in the scanf alignment cases below, is justified.
+                struct {
+                        char buf[8];
+                        char canary;
+                } data;
+                static_assert( sizeof(data.buf) == 8 );
+                static_assert( &data.buf[8] == &data.canary );  // canary comes right after buf
+
+                void rep(const char* casename) {
+                        data.canary = 42;
+                        bool caught = false;
+                        try {
+                                sin | wdi( sizeof(data.buf), data.buf );
+                        } catch (cstring_length*) {
+                                caught = true;
+                        }
+                        printf( "%s \"%s\"", casename, data.buf );
+                        if ( caught ) {
+                                printf(", exception occurred");
+                        }
+                        if ( data.canary == 42 ) {
+                                printf(", canary ok");
+                        } else {
+                                printf(", canary overwritten to %d", data.canary);
+                        }
+                        printf("\n");
+                }
+
+                rep("pre1");
+                rep("pre2a");
+                rep("pre2b");
+                scanf("\n");  // next test does not start with %s so does not tolerate leading whitespace
+        }
         {
                 char s[] = "yyyyyyyyyyyyyyyyyyyy";