On Sun, 22 Sep 2013 12:36:48 +0200 Peter Bex <peter....@xs4all.nl> wrote:
> 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.
Thanks a lot, Peter.
Unfortunately, it seems that it doesn't fix #1045. I still can
reproduce the crashes with awful-picman.
Your patch fixes a bug, so I think it should be pushed. I'm attaching
an edited version with a slight modification to avoid calling
(##sys#size dest) twice in certain code paths (as we discussed on IRC).
I also edited the commit message to remove the "Fix #1045" part.
I haven't pushed it because I edited it. The edited version is signed
off, so feel free to push it if you agree with the changes.
Best wishes.
Mario
--
http://parenteses.org/mario
>From 366e3eb5e75adaf2838c2fff8f5bfd39ad20ad3e Mon Sep 17 00:00:00 2001
From: Peter Bex <peter....@xs4all.nl>
Date: Sun, 22 Sep 2013 11:37:09 +0200
Subject: [PATCH] Read no more than the buffer length when a length of #f is
passed in
Signed-off-by: Mario Domenech Goulart <mario.goul...@gmail.com>
---
NEWS | 1 +
extras.scm | 8 ++++----
tests/port-tests.scm | 44 +++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 48 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..8f17e1f 100644
--- a/extras.scm
+++ b/extras.scm
@@ -176,10 +176,10 @@
(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!))
+ (let ((dest-size (##sys#size dest)))
+ (unless (and n (fx<= (fx+ start n) dest-size))
+ (set! n (fx- dest-size 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.7.10.4
_______________________________________________
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers
