Hi all,

I managed to figure out the cause behind *one* of the panics in #1045.
The manual says "read-string! reads destructively into the given STRING
argument, but never more characters than would fit into STRING".
See http://wiki.call-cc.org/man/4/Unit%20extras#read-string

Unfortunately, this is not always true: when you pass it #f for the
NUM argument, it will read until EOF, regardless of the size of the
buffer that's passed in.

Since this is a buffer overrun error with a reasonably simple fix,
I think this should go into the stability branch and an emergency
stability release should probably be made.

Attached is the patch which fixes it.  Any external code that's using
read-string!  should be investigated for #f arguments and fixed to
explicitly pass the buffer size, so that it won't cause trouble with
older, unfixed CHICKENs.  I'll modify http-client ASAP.

It would be great if Mario and Alaric could check whether this fix
solves the issues in awful-picman and Ugarit.  I was unable to reproduce
the awful-picman bug and the tests for Ugarit just cause so many errors
on my machine that I'm unsure what's going on.

Cheers,
Peter
-- 
http://www.more-magic.net
>From d13bf643f1fa5d5fd7e9e3a00b4720f1155ff508 Mon Sep 17 00:00:00 2001
From: Peter Bex <peter....@xs4all.nl>
Date: Sun, 22 Sep 2013 11:37:09 +0200
Subject: [PATCH] Fix #1045 by reading no more than the buffer length when a
 length of #f is passed in

---
 NEWS                 |  1 +
 extras.scm           |  7 +++----
 tests/port-tests.scm | 44 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 5b3cfdb..17560ce 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,7 @@
     to Florian Zumbiehl)
   - CVE-2013-2075: Use POSIX poll() in other places where select() was
     still being used.  (thanks to Florian Zumbiehl and Joerg Wittenberger)
+  - read-string! no longer reads beyond its buffer when length is #f.
 
 - Tools
   - csc: added "-oi"/"-ot" options as alternatives to "-emit-inline-file"
diff --git a/extras.scm b/extras.scm
index 49ab5cf..99a8470 100644
--- a/extras.scm
+++ b/extras.scm
@@ -176,10 +176,9 @@
 (define (read-string! n dest #!optional (port ##sys#standard-input) (start 0))
   (##sys#check-input-port port #t 'read-string!)
   (##sys#check-string dest 'read-string!)
-  (when n
-    (##sys#check-exact n 'read-string!)
-    (when (fx> (fx+ start n) (##sys#size dest))
-      (set! n (fx- (##sys#size dest) start))))
+  (when n (##sys#check-exact n 'read-string!))
+  (unless (and n (fx<= (fx+ start n) (##sys#size dest)))
+    (set! n (fx- (##sys#size dest) start)))
   (##sys#check-exact start 'read-string!)
   (##sys#read-string! n dest port start) )
 
diff --git a/tests/port-tests.scm b/tests/port-tests.scm
index 409c552..bd7c858 100644
--- a/tests/port-tests.scm
+++ b/tests/port-tests.scm
@@ -295,7 +295,49 @@ EOF
 
 (test-group
  "read-line string port position tests"
-(test-port-position read-string-line/pos))
+ (test-port-position read-string-line/pos))
+
+(test-group "read-string!"
+  (let ((in (open-input-string "1234567890"))
+        (buf (make-string 5)))
+    (test-equal "read-string! won't read past buffer if given #f"
+                (read-string! #f buf in)
+                5)
+    (test-equal "read-string! reads the requested bytes with #f"
+                buf
+                "12345")
+    (test-equal "read-string! won't read past buffer if given #f and offset"
+                (read-string! #f buf in 3)
+                2)
+    (test-equal "read-string! reads the requested bytes with #f and offset"
+                buf
+                "12367")
+    (test-equal "read-string! reads until the end correctly"
+                (read-string! #f buf in)
+                3)
+    (test-equal "read-string! leaves the buffer's tail intact"
+                buf
+                "89067"))
+  (let ((in (open-input-string "1234567890"))
+        (buf (make-string 5)))
+    (test-equal "read-string! won't read past buffer if given size"
+                (read-string! 10 buf in)
+                5)
+    (test-equal "read-string! reads the requested bytes with buffer size"
+                buf
+                "12345")
+    (test-equal "read-string! won't read past buffer if given size and offset"
+                (read-string! 10 buf in 3)
+                2)
+    (test-equal "read-string! reads the requested bytes with buffer size and 
offset"
+                buf
+                "12367")
+    (test-equal "read-string! reads until the end correctly with buffer size"
+                (read-string! 10 buf in)
+                3)
+    (test-equal "read-string! leaves the buffer's tail intact"
+                buf
+                "89067")))
 
 ;; Disabled because it requires `echo -n` for
 ;; the EOF test, and that is not available on all systems.
-- 
1.8.3.4

_______________________________________________
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Reply via email to