Faculty of Information Technology
Software Engineering Group

Ticket #197: libbasic_fix_1_of_1_rev_cd0581d5639b_Issue__197__fixed_buffer_overflow_in___makeWchar___.patch

File libbasic_fix_1_of_1_rev_cd0581d5639b_Issue__197__fixed_buffer_overflow_in___makeWchar___.patch, 3.0 KB (added by Jan Vrany, 20 months ago)
  • Win32OperatingSystem.st

    # HG changeset patch
    # User Jan Vrany <jan.vrany@fit.cvut.cz>
    # Date 1522227118 -3600
    #      Wed Mar 28 09:51:58 2018 +0100
    # Branch jv
    # Node ID cd0581d5639bd0608c7e6aae7aa486ce6bd82dd4
    # Parent  4e9bbc6019e0e8d7a8867880ebbe986975129d3f
    Issue #197: fixed buffer overflow in `_makeWchar()`
    
    ...when passed string is `Unicode16String`. It (wrongly) used
    passed buffer size as size of the buffer in character while
    caller passed it as a size in bytes (a result of `sizeof()`).
    
    This naturally caused a buffer overflow when size of (unicode)
    string was greater (MAXPATHLEN / 2), causing the segmentation
    violation.
    
    This commit fixes this mismatch and makes sure that once
    `_makeWchar()` returns, it's properly NULL terminated.
    
    https://swing.fit.cvut.cz/projects/stx-jv/ticket/197
    
    diff -r 4e9bbc6019e0 -r cd0581d5639b Win32OperatingSystem.st
    a b  
    586586
    587587!Win32OperatingSystem primitiveFunctions!
    588588%{
    589 int
    590 _makeWchar(OBJ string, wchar_t *buffer, int bufferSize)
     589
     590/**
     591 * The `_makeWchar()` function copies contents of a string object `srcObj` to a
     592 * (wide char) buffer `dst`. At most `n` **bytes** including null terminator
     593 * are copied to destination buffer `dst`.
     594 *
     595 * Returns the number of **bytes** copied (including null terminator). If
     596 * source string object is not an instance of `String` or `Unicode16String`,
     597 * returns `-1`.
     598 */
     599static int
     600_makeWchar(OBJ srcObj, wchar_t *dst, int n/* in bytes!!! */)
    591601{
    592     int i, len;
    593 
    594     if (__isStringLike(string)) {
    595         len = __stringSize(string);
    596         if (len > bufferSize) len = bufferSize;
    597         for (i=0; i<len; i++) {
    598             buffer[i] = __stringVal(string)[i];
    599         }
    600     } else if (__isUnicode16String(string)) {
    601         len = __unicode16StringSize(string);
    602         if (len > bufferSize) len = bufferSize;
    603         for (i=0; i<len; i++) {
    604             buffer[i] = __unicode16StringVal(string)[i];
    605         }
     602    int i;
     603    int srcLen; /* length of string to be copied, in **characters** */
     604    int dstLen; /* length of destination buffer,  in **characters** */
     605
     606    dstLen = n / sizeof(wchar_t);
     607       
     608    if (__isStringLike(srcObj)) {
     609        srcLen = __stringSize(srcObj); 
     610       
     611        if (srcLen >= dstLen) srcLen = dstLen - 1;
     612
     613        for (i=0; i<srcLen; i++) {
     614            dst[i] = __stringVal(srcObj)[i];
     615        }
     616    } else if (__isUnicode16String(srcObj)) {           
     617        srcLen = __unicode16StringSize(srcObj);
     618       
     619        if (srcLen >= dstLen) srcLen = dstLen - 1;             
     620       
     621        memcpy(dst, __unicode16StringVal(srcObj), srcLen * sizeof(wchar_t));
    606622    } else {
     623        dst[0] = 0;
    607624        return(-1);
    608625    }
    609     buffer[len] = 0;
    610     return(len);
     626    dst[srcLen] = 0;   
     627    return(srcLen * sizeof(wchar_t));
    611628}
    612629
    613630
     
    67006717    if (__isSmallInteger(anInteger)) {
    67016718        if (__isStringLike(aPathName)) {
    67026719#ifdef DO_WRAP_CALLS
    6703             char _aPathName[MAXPATHLEN];
     6720            char _aPathName[MAXPATHLEN+1];
    67046721
    67056722            strncpy(_aPathName, __stringVal(aPathName), MAXPATHLEN-1); _aPathName[MAXPATHLEN-1] = '\0';
    67066723            do {